Skip to content

Commit 7ca165a

Browse files
author
vijay
committed
re validate the outbound payload for mutation
1 parent 488e706 commit 7ca165a

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

‎src/mcp/server/skills.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,24 @@ def methods(self) -> Sequence[MethodBinding]:
104104
async def _handle_list(self, ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> HandlerResult:
105105
# `ListSkillsResult`/`Skill` self-validate on construction, so a handler that builds a
106106
# non-conformant listing raises `ValidationError` here — a server fault, surfaced as an
107-
# Internal error rather than the framework's default Invalid params for a bad body.
107+
# Internal error rather than the framework's default Invalid params for a bad body. The
108+
# models stay mutable, so re-validate the outbound payload too: a handler that mutates a
109+
# skill after building the result can't ship a non-conformant listing past this point.
108110
try:
109111
result = await self._list_skills(ctx, params)
112+
ListSkillsResult.model_validate(result.model_dump())
110113
except ValidationError:
111114
logger.exception("list_skills handler returned an invalid result")
112115
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
113116
return _finalize_cacheable(result, ctx.protocol_version)
114117

115118
async def _handle_get(self, ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> HandlerResult:
116119
_require_skill_md_uri(params.uri)
117-
# `Skill` self-validates on construction (see `_handle_list`).
120+
# `Skill` self-validates on construction, and the re-validation guards post-construction
121+
# mutation, both as in `_handle_list`.
118122
try:
119123
result = await self._get_skill(ctx, params)
124+
GetSkillResult.model_validate(result.model_dump())
120125
except ValidationError:
121126
logger.exception("get_skill handler returned an invalid result")
122127
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None

‎tests/server/test_skills.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,23 @@ async def bad_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams
150150
assert exc_info.value.code == INTERNAL_ERROR
151151

152152

153+
async def test_skills_list_rejects_a_skill_mutated_after_the_result_is_built() -> None:
154+
"""SDK-defined: the value models validate on construction but stay mutable. A handler that
155+
builds a valid result and then mutates a skill in place must not slip a non-conformant
156+
listing past the server — the outbound payload is re-validated, so this is an Internal error."""
157+
158+
async def mutating_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult:
159+
result = ListSkillsResult(skills=[_git_workflow_skill()])
160+
result.skills[0].frontmatter["name"] = "not the uri name"
161+
return result
162+
163+
server = MCPServer("catalog", extensions=[Skills(list_skills=mutating_list, get_skill=_get_skill)])
164+
async with Client(server) as client:
165+
with pytest.raises(MCPError) as exc_info:
166+
await client.session.send_request(ListSkillsRequest(), ListSkillsResult)
167+
assert exc_info.value.code == INTERNAL_ERROR
168+
169+
153170
async def test_skills_get_returns_the_matching_skill() -> None:
154171
async with Client(_server()) as client:
155172
result = await client.session.send_request(
@@ -247,6 +264,23 @@ async def bad_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams)
247264
assert exc_info.value.code == INTERNAL_ERROR
248265

249266

267+
async def test_skills_get_rejects_a_skill_mutated_after_the_result_is_built() -> None:
268+
"""SDK-defined: as on `skills/list`, a handler that mutates the skill in place after building
269+
the result can't ship a non-conformant body — the re-validated outbound payload fails it as an
270+
Internal error."""
271+
272+
async def mutating_get(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult:
273+
result = GetSkillResult(skill=_git_workflow_skill())
274+
result.skill.resources.clear()
275+
return result
276+
277+
server = MCPServer("catalog", extensions=[Skills(list_skills=_list_skills, get_skill=mutating_get)])
278+
async with Client(server) as client:
279+
with pytest.raises(MCPError) as exc_info:
280+
await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult)
281+
assert exc_info.value.code == INTERNAL_ERROR
282+
283+
250284
async def test_missing_uri_param_is_rejected_before_the_handler_runs() -> None:
251285
"""SDK-defined: `uri` is a required field on `GetSkillParams`, so an omitted param is
252286
rejected by params validation - the handler never sees it."""

0 commit comments

Comments
 (0)