Verify skill resources through the client extension

Author: Kludex

docs/advanced/skills.md

@@ -66,17 +66,17 @@ These three validate the server's response against the SEP-2640 conformance rule
 it. A name that doesn't match its URI, a digest in the wrong shape, or an incomplete manifest raises
 `ValueError` rather than reaching your code.
 
-`read_skill_uri` is the exception. It's a thin, discoverable alias for `resources/read` that returns
-a `ReadResourceResult` (text or blob contents) and validates nothing itself — that's the next step.
+`read_skill_resource(skill, uri)` fetches a file and verifies it against the manifest in the `Skill`
+entry you pass. It returns a `ReadResourceResult` with text or blob content only after the URI,
+size, and SHA-256 digest match. Keep that entry while you act on the skill so every file is checked
+against the same manifest.
 
 !!! tip
-    `verify_skill_resource(skill, uri, content)` checks a file's bytes — size, then SHA-256
-    digest — against the entry you hold for it. Call it after `read_skill_uri` and *before* you
-    treat the content as trustworthy.
-
-    `resources/read` returns whatever bytes the server sends *right now*; verification is what ties
-    those bytes back to the manifest you already validated. It applies to a static manifest only —
-    a `"dynamic"` skill carries no digests, so calling it on one raises `ValueError`.
+    `read_skill_resource` checks the bytes returned by `resources/read` against the held manifest.
+    A `"dynamic"` skill carries no digests, so this method raises `ValueError` for one. You can use
+    `Client.read_resource` to fetch dynamic content, but it cannot be integrity-verified.
+    If you fetch or cache bytes outside this method, `mcp.shared.skills.verify_skill_resource`
+    performs the same check on those bytes.
 
 !!! warning
     Skill content is untrusted model input, exactly like any other server-provided text. SEP-2640

docs_src/skills/tutorial001_client.py

@@ -2,7 +2,6 @@
 
 from mcp import Client
 from mcp.client.skills import Skills
-from mcp.shared.skills import verify_skill_resource
 from mcp.types import TextResourceContents
 
 
@@ -13,10 +12,9 @@ async def main() -> None:
             print(skill.uri, skill.frontmatter["description"])
 
         skill = await catalog.get_skill("skill://git-workflow/SKILL.md")
-        result = await catalog.read_skill_uri(skill.uri)
+        result = await catalog.read_skill_resource(skill, skill.uri)
         content = result.contents[0]
         if isinstance(content, TextResourceContents):
-            verify_skill_resource(skill, skill.uri, content.text.encode())
             print(content.text)
 
 

src/mcp/client/skills.py

@@ -3,7 +3,7 @@
 `Skills` is an opt-in [`ClientExtension`](../advanced/extensions.md) for talking
 to a skills catalog server. Register it with `Client(extensions=[Skills()])`,
 then call `client.extension(Skills)` for the SEP-2640 verbs — `list_skills`, `get_skill`,
-`read_skill_uri`, and `read_directory` — tied to that connection:
+`read_skill_resource`, and `read_directory` — tied to that connection:
 
     async with Client("http://localhost:8000/mcp", extensions=[Skills()]) as client:
         for skill in await client.extension(Skills).list_skills():
@@ -13,13 +13,15 @@
 `get_skill`, and `read_directory` — check that the server advertises the
 extension and validate its response; `list_skills` and `read_directory` follow
 `nextCursor` to completion, so one call returns every page's results.
-`read_skill_uri` is a thin `resources/read` alias that does neither — verify its
-result with `verify_skill_resource`.
+`read_skill_resource` reads one file and verifies it against the held `Skill` entry.
 """
 
 from __future__ import annotations
 
-from mcp_types import ReadResourceResult, Resource
+import base64
+import binascii
+
+from mcp_types import ReadResourceResult, Resource, TextResourceContents
 
 from mcp.client.extension import ClientExtension
 from mcp.client.session import ClientSession
@@ -36,6 +38,7 @@
     ReadDirectoryResult,
     Skill,
     validate_directory_result,
+    verify_skill_resource,
 )
 
 __all__ = ["BoundSkills", "Skills"]
@@ -63,9 +66,8 @@ class BoundSkills:
     `get_skill`, and `read_directory` — check that the server advertises the
     extension and validate its response against the SEP-2640 conformance rules
     before returning; `list_skills` and `read_directory` also follow
-    `nextCursor` to completion. `read_skill_uri` is the exception: a thin
-    `resources/read` alias that neither checks advertisement nor validates —
-    pair it with `verify_skill_resource`.
+    `nextCursor` to completion. `read_skill_resource` verifies a fetched file
+    against the held skill's manifest.
     """
 
     def __init__(self, session: ClientSession) -> None:
@@ -136,20 +138,37 @@ async def get_skill(self, uri: str) -> Skill:
             raise ValueError(f"server returned skill {result.skill.uri!r} for requested {uri!r}")
         return result.skill
 
-    async def read_skill_uri(self, uri: str) -> ReadResourceResult:
-        """Read a skill file's content via `resources/read`.
+    async def read_skill_resource(self, skill: Skill, uri: str) -> ReadResourceResult:
+        """Read and verify one file against a held `Skill` entry.
 
-        A thin, discoverable alias: works for any `skill://` (or other-scheme)
-        file regardless of whether the skill was ever enumerated. Verify the
-        result against a held `Skill` entry with `verify_skill_resource` before
-        treating it as trusted content — this call does not verify anything itself.
+        Pass the entry obtained from `list_skills` or `get_skill`. A dynamic
+        skill has no manifest to verify; use `Client.read_resource` if you
+        choose to read its unverified content.
 
         Raises:
+            ValueError: If the file is not in the manifest, or the server's
+                content does not match its URI, size, or digest.
             MCPError: If the server returns an error response.
             RuntimeError: If the server returns an `InputRequiredResult`; this
-                alias does not drive the input-required loop.
+                method does not drive the input-required loop.
         """
-        return await self._session.read_resource(uri)
+        if skill.resources == "dynamic":
+            raise ValueError(f"skill {skill.uri!r} has dynamic resources and cannot be integrity-verified")
+        if not any(resource.uri == uri for resource in skill.resources):
+            raise ValueError(f"{uri!r} is not in skill {skill.uri!r}'s held manifest")
+        result = await self._session.read_resource(uri)
+        if len(result.contents) != 1 or result.contents[0].uri != uri:
+            raise ValueError(f"resource {uri!r} did not return exactly one matching content item")
+        content = result.contents[0]
+        if isinstance(content, TextResourceContents):
+            data = content.text.encode("utf-8")
+        else:
+            try:
+                data = base64.b64decode(content.blob, validate=True)
+            except binascii.Error as exc:
+                raise ValueError(f"resource {uri!r} returned invalid base64 content") from exc
+        verify_skill_resource(skill, uri, data)
+        return result
 
     async def read_directory(self, uri: str, params: ReadDirectoryParams | None = None) -> list[Resource]:
         """Call `resources/directory/read` for `uri`, following `nextCursor` to completion.

tests/client/test_skills.py

@@ -1,20 +1,30 @@
 """Tests for the client-side Skills extension (SEP-2640, `mcp.client.skills`)."""
 
+import base64
 import hashlib
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
 from typing import Any
 
 import pytest
-from mcp_types import INVALID_PARAMS, Resource, TextResourceContents
+from inline_snapshot import snapshot
+from mcp_types import (
+    INVALID_PARAMS,
+    BlobResourceContents,
+    ReadResourceRequestParams,
+    ReadResourceResult,
+    Resource,
+    TextResourceContents,
+)
 
 from mcp.client.client import Client
 from mcp.client.skills import BoundSkills
 from mcp.client.skills import Skills as ClientSkills
+from mcp.server import Server
 from mcp.server.context import ServerRequestContext
 from mcp.server.extension import Extension, MethodBinding
 from mcp.server.mcpserver import MCPServer
-from mcp.server.mcpserver.resources import TextResource
+from mcp.server.mcpserver.resources import FunctionResource, TextResource
 from mcp.server.skills import Skills
 from mcp.shared.exceptions import MCPError
 from mcp.shared.skills import (
@@ -28,7 +38,6 @@
     ReadDirectoryResult,
     Skill,
     SkillResource,
-    verify_skill_resource,
 )
 
 pytestmark = pytest.mark.anyio
@@ -196,22 +205,117 @@ async def test_get_skill_propagates_the_servers_unknown_skill_error() -> None:
     assert exc_info.value.code == INVALID_PARAMS
 
 
-async def test_read_skill_uri_reads_the_registered_resource() -> None:
+async def test_read_skill_resource_returns_verified_text() -> None:
+    """SDK-defined: a matching manifest allows a fetched text file."""
     async with _skills(_server()) as skills:
-        result = await skills.read_skill_uri(_SKILL_URI)
+        skill = await skills.get_skill(_SKILL_URI)
+        result = await skills.read_skill_resource(skill, _SKILL_URI)
     contents = result.contents[0]
     assert isinstance(contents, TextResourceContents)
     assert contents.text == _SKILL_CONTENT
 
 
-async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None:
-    """End-to-end: `skills/get`'s digest and `resources/read`'s bytes agree."""
+async def test_read_skill_resource_rejects_a_changed_file() -> None:
+    """SEP-2640: a host rejects content that differs from the held manifest."""
+    stale = Skill(
+        uri=_SKILL_URI,
+        frontmatter={"name": "git-workflow", "description": "d"},
+        resources=[SkillResource(uri=_SKILL_URI, digest="sha256:" + "0" * 64, size=len(_SKILL_CONTENT))],
+    )
     async with _skills(_server()) as skills:
-        skill = await skills.get_skill(_SKILL_URI)
-        result = await skills.read_skill_uri(_SKILL_URI)
+        with pytest.raises(ValueError) as exc_info:
+            await skills.read_skill_resource(stale, _SKILL_URI)
+    assert str(exc_info.value) == snapshot(
+        "resource 'skill://git-workflow/SKILL.md' has digest "
+        "'sha256:7187b725da92d7447b6f50e21d0bf4f3b9db3387acb39239c4a4edc193ddcd4b', "
+        "expected 'sha256:0000000000000000000000000000000000000000000000000000000000000000'"
+    )
+
+
+async def test_read_skill_resource_rejects_an_unlisted_uri() -> None:
+    """SEP-2640: the host must not fetch a URI absent from the held manifest."""
+    async with _skills(_server()) as skills:
+        with pytest.raises(ValueError) as exc_info:
+            await skills.read_skill_resource(_skill(), "skill://git-workflow/unlisted.md")
+    assert str(exc_info.value) == snapshot(
+        "'skill://git-workflow/unlisted.md' is not in skill 'skill://git-workflow/SKILL.md''s held manifest"
+    )
+
+
+async def test_read_skill_resource_rejects_a_dynamic_skill() -> None:
+    """SDK-defined: a dynamic skill has no digest to verify against."""
+    dynamic = Skill(uri=_SKILL_URI, frontmatter={"name": "git-workflow", "description": "d"}, resources="dynamic")
+    async with _skills(_server()) as skills:
+        with pytest.raises(ValueError) as exc_info:
+            await skills.read_skill_resource(dynamic, _SKILL_URI)
+    assert str(exc_info.value) == snapshot(
+        "skill 'skill://git-workflow/SKILL.md' has dynamic resources and cannot be integrity-verified"
+    )
+
+
+async def test_read_skill_resource_returns_verified_binary_content() -> None:
+    """SDK-defined: binary resources are decoded before digest verification."""
+    data = b"\x00\x01skill\xff"
+    server = MCPServer("catalog")
+    server.add_resource(FunctionResource(uri=_SKILL_URI, name="SKILL.md", fn=lambda: data))
+    skill = Skill(
+        uri=_SKILL_URI,
+        frontmatter={"name": "git-workflow", "description": "d"},
+        resources=[SkillResource(uri=_SKILL_URI, digest=f"sha256:{hashlib.sha256(data).hexdigest()}", size=len(data))],
+    )
+    async with _skills(server) as skills:
+        result = await skills.read_skill_resource(skill, _SKILL_URI)
     contents = result.contents[0]
-    assert isinstance(contents, TextResourceContents)
-    verify_skill_resource(skill, _SKILL_URI, contents.text.encode())
+    assert isinstance(contents, BlobResourceContents)
+    assert contents.blob == base64.b64encode(data).decode("ascii")
+
+
+@pytest.mark.parametrize(
+    "contents",
+    [
+        [],
+        [TextResourceContents(uri="skill://other/SKILL.md", text=_SKILL_CONTENT)],
+        [TextResourceContents(uri=_SKILL_URI, text=_SKILL_CONTENT)] * 2,
+    ],
+)
+async def test_read_skill_resource_rejects_an_invalid_resource_response(
+    contents: list[TextResourceContents | BlobResourceContents],
+) -> None:
+    """SDK-defined: malformed peer responses cannot become verified content.
+
+    A low-level server is needed because `MCPServer` always produces conformant resource responses.
+    """
+
+    async def handler(ctx: ServerRequestContext[Any, Any], params: ReadResourceRequestParams) -> ReadResourceResult:
+        assert params.uri == _SKILL_URI
+        return ReadResourceResult(contents=contents)
+
+    server = Server("catalog")
+    server.add_request_handler("resources/read", ReadResourceRequestParams, handler)
+    async with Client(server, extensions=[ClientSkills()]) as client:
+        with pytest.raises(ValueError) as exc_info:
+            await client.extension(ClientSkills).read_skill_resource(_skill(), _SKILL_URI)
+    assert str(exc_info.value) == snapshot(
+        "resource 'skill://git-workflow/SKILL.md' did not return exactly one matching content item"
+    )
+
+
+async def test_read_skill_resource_rejects_invalid_base64() -> None:
+    """SDK-defined: a malformed binary response cannot become verified content.
+
+    A low-level server is needed because `MCPServer` always base64-encodes binary resources.
+    """
+
+    async def handler(ctx: ServerRequestContext[Any, Any], params: ReadResourceRequestParams) -> ReadResourceResult:
+        assert params.uri == _SKILL_URI
+        return ReadResourceResult(contents=[BlobResourceContents(uri=_SKILL_URI, blob="??")])
+
+    server = Server("catalog")
+    server.add_request_handler("resources/read", ReadResourceRequestParams, handler)
+    async with Client(server, extensions=[ClientSkills()]) as client:
+        with pytest.raises(ValueError) as exc_info:
+            await client.extension(ClientSkills).read_skill_resource(_skill(), _SKILL_URI)
+    assert str(exc_info.value) == snapshot("resource 'skill://git-workflow/SKILL.md' returned invalid base64 content")
 
 
 async def test_read_directory_follows_next_cursor_to_completion() -> None:

tests/docs_src/test_skills.py

@@ -7,7 +7,6 @@
 from mcp import Client
 from mcp.client.skills import Skills
 from mcp.shared.exceptions import MCPError
-from mcp.shared.skills import verify_skill_resource
 
 pytestmark = pytest.mark.anyio
 
@@ -48,14 +47,13 @@ async def test_the_skill_file_is_served_as_an_ordinary_resource() -> None:
     assert contents.text == tutorial001.SKILL_MD
 
 
-async def test_read_skill_uri_content_verifies_against_the_held_skill() -> None:
-    """tutorial001_client: fetch the entry, read the file, verify the bytes against it -
-    the digest/size check `verify_skill_resource` performs."""
+async def test_read_skill_resource_returns_verified_content() -> None:
+    """tutorial001_client: a fetched file matches the held skill's manifest."""
     skills = Skills()
     async with Client(tutorial001.mcp, extensions=[skills]) as client:
         catalog = client.extension(Skills)
         skill = await catalog.get_skill(tutorial001.SKILL_URI)
-        result = await catalog.read_skill_uri(skill.uri)
+        result = await catalog.read_skill_resource(skill, skill.uri)
     contents = result.contents[0]
     assert isinstance(contents, TextResourceContents)
-    verify_skill_resource(skill, skill.uri, contents.text.encode())
+    assert contents.text == tutorial001.SKILL_MD