Skip to content

Commit 7ff625f

Browse files
committed
Limit Skills client API to catalog methods
1 parent c7c4a95 commit 7ff625f

7 files changed

Lines changed: 17 additions & 154 deletions

File tree

‎docs/advanced/skills.md‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ There are three moves here:
4646
And that's it. `Skills(list_skills=..., get_skill=...)` is all a server needs;
4747
`resources/directory/read` is optional (more on that below).
4848

49-
## Fetching a skill
49+
## Inspecting a skill
5050

5151
On the client side, `Skills` is a [`ClientExtension`](extensions.md). You register it the same way
5252
you register any other one — by passing it to `Client(extensions=[...])` — and then call `extension` to
@@ -56,7 +56,7 @@ get the verbs tied to that connection:
5656
--8<-- "docs_src/skills/tutorial001_client.py"
5757
```
5858

59-
`client.extension(Skills)` hands you a `BoundSkills`, and its methods are the SEP-2640 verbs:
59+
`client.extension(Skills)` hands you a `BoundSkills` with the SEP-2640 methods:
6060

6161
* `list_skills` and `read_directory` follow `nextCursor` to completion, so a single call gives you
6262
every page's skills or resources.
@@ -66,25 +66,21 @@ These three validate the server's response against the SEP-2640 conformance rule
6666
it. A name that doesn't match its URI, a digest in the wrong shape, or an incomplete manifest raises
6767
`ValueError` rather than reaching your code.
6868

69-
`read_skill_uri` is the exception. It's a thin, discoverable alias for `resources/read` that returns
70-
a `ReadResourceResult` (text or blob contents) and validates nothing itself — that's the next step.
69+
Read a skill file with `Client.read_resource(uri)`, the same method you use for any MCP resource.
70+
It returns raw content without checking it against the skill's manifest.
7171

72-
!!! tip
73-
`verify_skill_resource(skill, uri, content)` checks a file's bytes — size, then SHA-256
74-
digest — against the entry you hold for it. Call it after `read_skill_uri` and *before* you
75-
treat the content as trustworthy.
72+
!!! warning "Verify files before loading"
73+
A host that loads a skill must hold its `Skill` entry. Before using a fetched file, check that
74+
its URI appears in that entry's `resources` and that its bytes match the advertised size and
75+
SHA-256 digest. A `"dynamic"` skill has no digest to check. Refreshing the entry to accept
76+
changed bytes also changes the content to which any prior approval applied.
7677

77-
`resources/read` returns whatever bytes the server sends *right now*; verification is what ties
78-
those bytes back to the manifest you already validated. It applies to a static manifest only —
79-
a `"dynamic"` skill carries no digests, so calling it on one raises `ValueError`.
80-
81-
!!! warning
8278
Skill content is untrusted model input, exactly like any other server-provided text. SEP-2640
8379
requires a host to tag it with its originating server before it reaches the model, and to
8480
never grant the frontmatter's `allowed-tools` field (or any other permission-widening field)
8581
without explicit per-skill user approval.
8682

87-
A digest match confirms the *bytes*, not the *frontmatter*: it doesn't prove the
83+
A matching digest confirms the *bytes*, not the *frontmatter*: it doesn't prove the
8884
`frontmatter` the server advertised in `skills/get` matches the frontmatter inside the fetched
8985
`SKILL.md`. If you act on `skill.frontmatter` — especially `allowed-tools` — parse the fetched
9086
file and compare its frontmatter yourself.

‎docs_src/skills/tutorial001_client.py‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
from mcp import Client
44
from mcp.client.skills import Skills
5-
from mcp.shared.skills import verify_skill_resource
6-
from mcp.types import TextResourceContents
75

86

97
async def main() -> None:
@@ -13,11 +11,7 @@ async def main() -> None:
1311
print(skill.uri, skill.frontmatter["description"])
1412

1513
skill = await catalog.get_skill("skill://git-workflow/SKILL.md")
16-
result = await catalog.read_skill_uri(skill.uri)
17-
content = result.contents[0]
18-
if isinstance(content, TextResourceContents):
19-
verify_skill_resource(skill, skill.uri, content.text.encode())
20-
print(content.text)
14+
print(skill.resources)
2115

2216

2317
if __name__ == "__main__":

‎src/mcp/client/skills.py‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
`Skills` is an opt-in [`ClientExtension`](../advanced/extensions.md) for talking
44
to a skills catalog server. Register it with `Client(extensions=[Skills()])`,
55
then call `client.extension(Skills)` for the SEP-2640 verbs — `list_skills`, `get_skill`,
6-
`read_skill_uri`, and `read_directory` — tied to that connection:
6+
and `read_directory` — tied to that connection:
77
88
async with Client("http://localhost:8000/mcp", extensions=[Skills()]) as client:
99
for skill in await client.extension(Skills).list_skills():
@@ -13,13 +13,11 @@
1313
`get_skill`, and `read_directory` — check that the server advertises the
1414
extension and validate its response; `list_skills` and `read_directory` follow
1515
`nextCursor` to completion, so one call returns every page's results.
16-
`read_skill_uri` is a thin `resources/read` alias that does neither — verify its
17-
result with `verify_skill_resource`.
1816
"""
1917

2018
from __future__ import annotations
2119

22-
from mcp_types import ReadResourceResult, Resource
20+
from mcp_types import Resource
2321

2422
from mcp.client.extension import ClientExtension
2523
from mcp.client.session import ClientSession
@@ -63,9 +61,8 @@ class BoundSkills:
6361
`get_skill`, and `read_directory` — check that the server advertises the
6462
extension and validate its response against the SEP-2640 conformance rules
6563
before returning; `list_skills` and `read_directory` also follow
66-
`nextCursor` to completion. `read_skill_uri` is the exception: a thin
67-
`resources/read` alias that neither checks advertisement nor validates —
68-
pair it with `verify_skill_resource`.
64+
`nextCursor` to completion. Read a skill's files with the ordinary
65+
`Client.read_resource` method.
6966
"""
7067

7168
def __init__(self, session: ClientSession) -> None:
@@ -136,21 +133,6 @@ async def get_skill(self, uri: str) -> Skill:
136133
raise ValueError(f"server returned skill {result.skill.uri!r} for requested {uri!r}")
137134
return result.skill
138135

139-
async def read_skill_uri(self, uri: str) -> ReadResourceResult:
140-
"""Read a skill file's content via `resources/read`.
141-
142-
A thin, discoverable alias: works for any `skill://` (or other-scheme)
143-
file regardless of whether the skill was ever enumerated. Verify the
144-
result against a held `Skill` entry with `verify_skill_resource` before
145-
treating it as trusted content — this call does not verify anything itself.
146-
147-
Raises:
148-
MCPError: If the server returns an error response.
149-
RuntimeError: If the server returns an `InputRequiredResult`; this
150-
alias does not drive the input-required loop.
151-
"""
152-
return await self._session.read_resource(uri)
153-
154136
async def read_directory(self, uri: str, params: ReadDirectoryParams | None = None) -> list[Resource]:
155137
"""Call `resources/directory/read` for `uri`, following `nextCursor` to completion.
156138

‎src/mcp/shared/skills.py‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
from __future__ import annotations
99

10-
import hashlib
1110
import re
1211
from typing import Annotated, Any, Literal
1312
from urllib.parse import urlsplit
@@ -27,7 +26,6 @@
2726
"GetSkillResult",
2827
"ReadDirectoryParams",
2928
"ReadDirectoryResult",
30-
"verify_skill_resource",
3129
]
3230

3331
EXTENSION_ID = "io.modelcontextprotocol/skills"
@@ -297,27 +295,3 @@ def validate_directory_result(uri: str, result: ReadDirectoryResult) -> None:
297295
if resource.uri in seen_uris:
298296
raise ValueError(f"directory {uri!r} contains a duplicate child {resource.uri!r}")
299297
seen_uris.add(resource.uri)
300-
301-
302-
def verify_skill_resource(skill: Skill, uri: str, content: bytes) -> None:
303-
"""Check that `content` (the bytes read from `uri`) matches `skill`'s manifest entry.
304-
305-
Recomputes the size and SHA-256 digest of `content` and compares them to the
306-
entry `skill` holds for `uri` — the byte-integrity check SEP-2640 requires
307-
before a host trusts a fetched file. A `"dynamic"` skill carries no digests,
308-
so it has nothing to verify against.
309-
310-
Raises:
311-
ValueError: If `uri` is not one of `skill`'s resources, `skill.resources`
312-
is `"dynamic"`, or `content`'s size or digest doesn't match the entry.
313-
"""
314-
if skill.resources == "dynamic":
315-
raise ValueError(f"skill {skill.uri!r} has dynamic resources and cannot be integrity-verified")
316-
entry = next((r for r in skill.resources if r.uri == uri), None)
317-
if entry is None:
318-
raise ValueError(f"{uri!r} is not in skill {skill.uri!r}'s held manifest")
319-
if len(content) != entry.size:
320-
raise ValueError(f"resource {uri!r} has size {len(content)}, expected {entry.size}")
321-
digest = f"sha256:{hashlib.sha256(content).hexdigest()}"
322-
if digest != entry.digest:
323-
raise ValueError(f"resource {uri!r} has digest {digest!r}, expected {entry.digest!r}")

‎tests/client/test_skills.py‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from typing import Any
77

88
import pytest
9-
from mcp_types import INVALID_PARAMS, Resource, TextResourceContents
9+
from mcp_types import INVALID_PARAMS, Resource
1010

1111
from mcp.client.client import Client
1212
from mcp.client.skills import BoundSkills
@@ -28,7 +28,6 @@
2828
ReadDirectoryResult,
2929
Skill,
3030
SkillResource,
31-
verify_skill_resource,
3231
)
3332

3433
pytestmark = pytest.mark.anyio
@@ -196,24 +195,6 @@ async def test_get_skill_propagates_the_servers_unknown_skill_error() -> None:
196195
assert exc_info.value.code == INVALID_PARAMS
197196

198197

199-
async def test_read_skill_uri_reads_the_registered_resource() -> None:
200-
async with _skills(_server()) as skills:
201-
result = await skills.read_skill_uri(_SKILL_URI)
202-
contents = result.contents[0]
203-
assert isinstance(contents, TextResourceContents)
204-
assert contents.text == _SKILL_CONTENT
205-
206-
207-
async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None:
208-
"""End-to-end: `skills/get`'s digest and `resources/read`'s bytes agree."""
209-
async with _skills(_server()) as skills:
210-
skill = await skills.get_skill(_SKILL_URI)
211-
result = await skills.read_skill_uri(_SKILL_URI)
212-
contents = result.contents[0]
213-
assert isinstance(contents, TextResourceContents)
214-
verify_skill_resource(skill, _SKILL_URI, contents.text.encode())
215-
216-
217198
async def test_read_directory_follows_next_cursor_to_completion() -> None:
218199
async with _skills(_server(with_directory_read=True)) as skills:
219200
resources = await skills.read_directory("skill://git-workflow/references")

‎tests/docs_src/test_skills.py‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from mcp import Client
88
from mcp.client.skills import Skills
99
from mcp.shared.exceptions import MCPError
10-
from mcp.shared.skills import verify_skill_resource
1110

1211
pytestmark = pytest.mark.anyio
1312

@@ -46,16 +45,3 @@ async def test_the_skill_file_is_served_as_an_ordinary_resource() -> None:
4645
assert isinstance(contents, TextResourceContents)
4746
assert contents.mime_type == "text/markdown"
4847
assert contents.text == tutorial001.SKILL_MD
49-
50-
51-
async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None:
52-
"""tutorial001_client: fetch the entry, read the file, verify the bytes against it -
53-
the digest/size check `verify_skill_resource` performs."""
54-
skills = Skills()
55-
async with Client(tutorial001.mcp, extensions=[skills]) as client:
56-
catalog = client.extension(Skills)
57-
skill = await catalog.get_skill(tutorial001.SKILL_URI)
58-
result = await catalog.read_skill_uri(skill.uri)
59-
contents = result.contents[0]
60-
assert isinstance(contents, TextResourceContents)
61-
verify_skill_resource(skill, skill.uri, contents.text.encode())

‎tests/shared/test_skills.py‎

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
"""SEP-2640 conformance checks in `mcp.shared.skills`: types, validation, and verification.
1+
"""SEP-2640 conformance checks in `mcp.shared.skills`: types and validation.
22
33
Server- and client-side end-to-end wiring live in `tests/server/test_skills.py` and
44
`tests/client/test_skills.py`; this file pins the pure functions both depend on.
55
"""
66

7-
import hashlib
87
from typing import Any
98

109
import pytest
@@ -21,7 +20,6 @@
2120
parse_directory_uri,
2221
skill_name_from_uri,
2322
validate_directory_result,
24-
verify_skill_resource,
2523
)
2624

2725
_DIGEST = "sha256:" + "a" * 64
@@ -356,51 +354,3 @@ def test_validate_directory_result_rejects_a_uri_outside_the_directory() -> None
356354
result = ReadDirectoryResult(resources=[Resource(uri="skill://other/file.md", name="file.md")])
357355
with pytest.raises(ValueError, match="not a child"):
358356
validate_directory_result("skill://pdf/templates", result)
359-
360-
361-
def test_verify_skill_resource_accepts_matching_content() -> None:
362-
content = b"hello skill"
363-
digest = f"sha256:{hashlib.sha256(content).hexdigest()}"
364-
uri = "skill://git-workflow/SKILL.md"
365-
skill = Skill(
366-
uri=uri,
367-
frontmatter={"name": "git-workflow", "description": "d"},
368-
resources=[SkillResource(uri=uri, digest=digest, size=len(content))],
369-
)
370-
verify_skill_resource(skill, uri, content)
371-
372-
373-
def test_verify_skill_resource_rejects_a_digest_mismatch() -> None:
374-
"""SEP-2640 Integrity and verification: a mismatch MUST be treated as a verification failure."""
375-
uri = "skill://git-workflow/SKILL.md"
376-
skill = Skill(
377-
uri=uri,
378-
frontmatter={"name": "git-workflow", "description": "d"},
379-
resources=[SkillResource(uri=uri, digest=_DIGEST, size=5)],
380-
)
381-
with pytest.raises(ValueError, match="digest"):
382-
verify_skill_resource(skill, uri, b"wrong")
383-
384-
385-
def test_verify_skill_resource_rejects_a_size_mismatch_before_hashing() -> None:
386-
uri = "skill://git-workflow/SKILL.md"
387-
skill = Skill(
388-
uri=uri,
389-
frontmatter={"name": "git-workflow", "description": "d"},
390-
resources=[SkillResource(uri=uri, digest=_DIGEST, size=999)],
391-
)
392-
with pytest.raises(ValueError, match="size"):
393-
verify_skill_resource(skill, uri, b"short")
394-
395-
396-
def test_verify_skill_resource_rejects_dynamic_resources() -> None:
397-
uri = "skill://generated/SKILL.md"
398-
skill = Skill(uri=uri, frontmatter={"name": "generated", "description": "d"}, resources="dynamic")
399-
with pytest.raises(ValueError, match="dynamic"):
400-
verify_skill_resource(skill, uri, b"anything")
401-
402-
403-
def test_verify_skill_resource_rejects_a_uri_not_in_the_manifest() -> None:
404-
skill = _skill()
405-
with pytest.raises(ValueError, match="not in skill"):
406-
verify_skill_resource(skill, "skill://git-workflow/unlisted.md", b"x")

0 commit comments

Comments
 (0)