fix(skills): unquote percent-encoded dot-segments during URI traversal validation

Author: chauvuusvn

src/mcp/shared/skills.py

@@ -9,7 +9,7 @@
 
 import re
 from typing import Annotated, Any, Literal
-from urllib.parse import urlsplit
+from urllib.parse import unquote, urlsplit
 
 from mcp_types import CacheableResult, PaginatedRequestParams, PaginatedResult, Request, RequestParams, Resource
 from pydantic import AfterValidator, BaseModel, ConfigDict, Field, model_validator
@@ -250,7 +250,8 @@ def _validate_resource_uri_in_skill(skill_uri: str, resource_uri: str) -> None:
     root = skill_parts.path[: -len("/SKILL.md")]
     if resource_parts.path != skill_parts.path and not resource_parts.path.startswith(root + "/"):
         raise ValueError(f"resource URI {resource_uri!r} is outside the skill root {skill_uri!r}")
-    if any(segment in (".", "..") for segment in resource_parts.path.split("/")):
+    unquoted_path = unquote(resource_parts.path)
+    if any(segment in (".", "..") for segment in unquoted_path.split("/")):
         raise ValueError(f"resource URI {resource_uri!r} contains a traversal segment")
 
 
@@ -290,7 +291,13 @@ def validate_directory_result(uri: str, result: ReadDirectoryResult) -> None:
         if child.scheme != scheme or child.netloc != netloc:
             raise ValueError(f"resource {resource.uri!r} is not a child of directory {uri!r}")
         relative = child.path.removeprefix(prefix)
-        if relative == child.path or not relative or "/" in relative or relative in (".", ".."):
+        unquoted_relative = unquote(relative)
+        if (
+            relative == child.path
+            or not relative
+            or "/" in unquoted_relative
+            or unquoted_relative in (".", "..")
+        ):
             raise ValueError(f"resource {resource.uri!r} is not a direct child of directory {uri!r}")
         if resource.uri in seen_uris:
             raise ValueError(f"directory {uri!r} contains a duplicate child {resource.uri!r}")

tests/shared/test_skills.py

@@ -149,9 +149,18 @@ def test_skill_rejects_a_resource_with_the_same_authority_but_a_sibling_path() -
         )
 
 
-def test_skill_rejects_a_traversal_segment_in_a_resource_uri() -> None:
+@pytest.mark.parametrize(
+    "evil_uri",
+    [
+        "skill://git-workflow/../evil.md",
+        "skill://git-workflow/%2e%2e/evil.md",
+        "skill://git-workflow/%2E%2E/evil.md",
+        "skill://git-workflow/%2e/evil.md",
+    ],
+)
+def test_skill_rejects_a_traversal_segment_in_a_resource_uri(evil_uri: str) -> None:
     with pytest.raises(ValidationError, match="traversal segment"):
-        _skill(extra_resources=[_resource("skill://git-workflow/../evil.md")])
+        _skill(extra_resources=[_resource(evil_uri)])
 
 
 def test_skill_rejects_a_resource_uri_with_a_trailing_slash() -> None:
@@ -342,7 +351,16 @@ def test_validate_directory_result_rejects_a_grandchild() -> None:
         validate_directory_result("skill://pdf/templates", result)
 
 
-@pytest.mark.parametrize("child_uri", ["skill://pdf/templates/.", "skill://pdf/templates/.."])
+@pytest.mark.parametrize(
+    "child_uri",
+    [
+        "skill://pdf/templates/.",
+        "skill://pdf/templates/..",
+        "skill://pdf/templates/%2e",
+        "skill://pdf/templates/%2e%2e",
+        "skill://pdf/templates/%2E%2E",
+    ],
+)
 def test_validate_directory_result_rejects_a_dot_segment_child(child_uri: str) -> None:
     """A `.`/`..` child is a traversal segment, not a real direct child of the directory."""
     result = ReadDirectoryResult(resources=[Resource(uri=child_uri, name="x")])