Skip to content

Commit 70493de

Browse files
author
vijay
committed
Reject directory-shaped skill resource URIs and dot-segment directory children
`_validate_resource_uri_in_skill` now rejects a resource URI ending in `/`, which names a directory rather than a file, and `validate_directory_result` now rejects a `.`/`..` child, which is a traversal segment rather than a real direct child. Both slipped past the prior checks.
1 parent 1adba1d commit 70493de

2 files changed

Lines changed: 18 additions & 1 deletion

File tree

‎src/mcp/shared/skills.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ def _validate_resource_uri_in_skill(skill_uri: str, resource_uri: str) -> None:
157157
resource_parts = urlsplit(resource_uri)
158158
if not resource_parts.scheme or resource_parts.query or resource_parts.fragment:
159159
raise ValueError(f"resource URI {resource_uri!r} is invalid")
160+
if resource_uri.endswith("/"):
161+
raise ValueError(f"resource URI {resource_uri!r} names a directory, not a file")
160162
if skill_parts.scheme != resource_parts.scheme or skill_parts.netloc != resource_parts.netloc:
161163
raise ValueError(f"resource URI {resource_uri!r} is outside the skill root {skill_uri!r}")
162164
root = skill_parts.path[: -len("/SKILL.md")]
@@ -257,7 +259,7 @@ def validate_directory_result(uri: str, result: ReadDirectoryResult) -> None:
257259
if child.scheme != scheme or child.netloc != netloc:
258260
raise ValueError(f"resource {resource.uri!r} is not a child of directory {uri!r}")
259261
relative = child.path.removeprefix(prefix)
260-
if relative == child.path or not relative or "/" in relative:
262+
if relative == child.path or not relative or "/" in relative or relative in (".", ".."):
261263
raise ValueError(f"resource {resource.uri!r} is not a direct child of directory {uri!r}")
262264
if resource.uri in seen_uris or resource.name in seen_names:
263265
raise ValueError(f"directory {uri!r} contains a duplicate child {resource.uri!r}")

‎tests/shared/test_skills.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,13 @@ def test_validate_skill_rejects_a_traversal_segment_in_a_resource_uri() -> None:
158158
validate_skill(skill)
159159

160160

161+
def test_validate_skill_rejects_a_resource_uri_with_a_trailing_slash() -> None:
162+
"""A manifest entry names a file, not a directory: a trailing slash is rejected."""
163+
skill = _skill(extra_resources=[_resource("skill://git-workflow/references/")])
164+
with pytest.raises(ValueError, match="names a directory"):
165+
validate_skill(skill)
166+
167+
161168
def test_validate_skill_rejects_a_negative_resource_size() -> None:
162169
root = "skill://git-workflow/SKILL.md"
163170
skill = Skill(
@@ -321,6 +328,14 @@ def test_validate_directory_result_rejects_a_grandchild() -> None:
321328
validate_directory_result("skill://pdf/templates", result)
322329

323330

331+
@pytest.mark.parametrize("child_uri", ["skill://pdf/templates/.", "skill://pdf/templates/.."])
332+
def test_validate_directory_result_rejects_a_dot_segment_child(child_uri: str) -> None:
333+
"""A `.`/`..` child is a traversal segment, not a real direct child of the directory."""
334+
result = ReadDirectoryResult(resources=[Resource(uri=child_uri, name="x")])
335+
with pytest.raises(ValueError, match="direct child"):
336+
validate_directory_result("skill://pdf/templates", result)
337+
338+
324339
def test_validate_directory_result_rejects_a_uri_outside_the_directory() -> None:
325340
result = ReadDirectoryResult(resources=[Resource(uri="skill://other/file.md", name="file.md")])
326341
with pytest.raises(ValueError, match="not a child"):

0 commit comments

Comments
 (0)