mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 15:19:10 +02:00
Fix percent-encoded skill file names unreadable in resources mode (#4590)
* Fix percent-encoded skill file names unreadable in resources mode Encode supporting-file paths explicitly (quote/unquote) when building and resolving skill:// resource URIs, instead of relying on AnyUrl's implicit encoding. This also closes the ambiguity where a file literally named "setup%20guide.md" would collide with "setup guide.md" once both were percent-encoded. Fixes #4545 * Quote main_file_name when building its resource URI Keeps the main-file URI on the same explicit quote/unquote round-trip as supporting files, so a custom main_file_name containing a literal '%' still resolves after the shared unquote() in _get_resource().
This commit is contained in:
parent
d0f1468fce
commit
30044c7864
2 changed files with 98 additions and 2 deletions
|
|
@ -7,6 +7,7 @@ import mimetypes
|
|||
from collections.abc import Sequence
|
||||
from pathlib import Path
|
||||
from typing import Any, Literal, cast
|
||||
from urllib.parse import quote, unquote
|
||||
|
||||
from mcp.shared.path_security import PathEscapeError, safe_join
|
||||
from pydantic import AnyUrl
|
||||
|
|
@ -287,7 +288,9 @@ class SkillProvider(Provider):
|
|||
# Main skill file
|
||||
resources.append(
|
||||
SkillResource(
|
||||
uri=AnyUrl(f"skill://{skill.name}/{self._main_file_name}"),
|
||||
uri=AnyUrl(
|
||||
f"skill://{skill.name}/{quote(self._main_file_name, safe='/')}"
|
||||
),
|
||||
name=f"{skill.name}/{self._main_file_name}",
|
||||
description=skill.description,
|
||||
mime_type="text/markdown",
|
||||
|
|
@ -318,7 +321,9 @@ class SkillProvider(Provider):
|
|||
mime_type, _ = mimetypes.guess_type(file_info.path)
|
||||
resources.append(
|
||||
SkillFileResource(
|
||||
uri=AnyUrl(f"skill://{skill.name}/{file_info.path}"),
|
||||
uri=AnyUrl(
|
||||
f"skill://{skill.name}/{quote(file_info.path, safe='/')}"
|
||||
),
|
||||
name=f"{skill.name}/{file_info.path}",
|
||||
description=f"File from {skill.name} skill",
|
||||
mime_type=mime_type or "application/octet-stream",
|
||||
|
|
@ -347,6 +352,7 @@ class SkillProvider(Provider):
|
|||
skill_name, file_path = parts
|
||||
if skill_name != skill.name:
|
||||
return None
|
||||
file_path = unquote(file_path)
|
||||
|
||||
if file_path == "_manifest":
|
||||
return SkillResource(
|
||||
|
|
|
|||
|
|
@ -176,6 +176,26 @@ This is my skill content.
|
|||
assert isinstance(result[0], TextResourceContents)
|
||||
assert "# My Skill" in result[0].text
|
||||
|
||||
async def test_read_main_file_with_literal_percent_in_name(self, tmp_path: Path):
|
||||
"""A custom main_file_name containing a literal '%' must round-trip
|
||||
through the same encode/decode path as supporting files (#4545)."""
|
||||
skill_dir = tmp_path / "percent-main-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "MAIN%20FILE.md").write_text("# Demo\n")
|
||||
|
||||
mcp = FastMCP("Test")
|
||||
mcp.add_provider(
|
||||
SkillProvider(skill_path=skill_dir, main_file_name="MAIN%20FILE.md")
|
||||
)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
resources = await client.list_resources()
|
||||
main = next(
|
||||
r for r in resources if r.name == "percent-main-skill/MAIN%20FILE.md"
|
||||
)
|
||||
result = await client.read_resource(main.uri)
|
||||
assert "# Demo" in result[0].text
|
||||
|
||||
async def test_read_manifest(self, single_skill_dir: Path):
|
||||
mcp = FastMCP("Test")
|
||||
mcp.add_provider(SkillProvider(skill_path=single_skill_dir))
|
||||
|
|
@ -230,6 +250,76 @@ This is my skill content.
|
|||
result = await client.read_resource(AnyUrl("skill://my-skill/reference.md"))
|
||||
assert "# Reference" in result[0].text
|
||||
|
||||
async def test_read_supporting_file_with_space_in_name(self, tmp_path: Path):
|
||||
"""Percent-encoded resource URIs for supporting files must round-trip (#4545)."""
|
||||
skill_dir = tmp_path / "space-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text("# Skill\n")
|
||||
(skill_dir / "setup guide.md").write_text("SPACE OK")
|
||||
|
||||
mcp = FastMCP("Test")
|
||||
mcp.add_provider(
|
||||
SkillProvider(skill_path=skill_dir, supporting_files="resources")
|
||||
)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
resources = await client.list_resources()
|
||||
supporting = next(
|
||||
r for r in resources if r.name == "space-skill/setup guide.md"
|
||||
)
|
||||
assert str(supporting.uri) == "skill://space-skill/setup%20guide.md"
|
||||
|
||||
result = await client.read_resource(supporting.uri)
|
||||
assert result[0].text == "SPACE OK"
|
||||
|
||||
async def test_read_supporting_file_with_utf8_name(self, tmp_path: Path):
|
||||
skill_dir = tmp_path / "utf8-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text("# Skill\n")
|
||||
(skill_dir / "café.md").write_text("UTF8 OK", encoding="utf-8")
|
||||
|
||||
mcp = FastMCP("Test")
|
||||
mcp.add_provider(
|
||||
SkillProvider(skill_path=skill_dir, supporting_files="resources")
|
||||
)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
resources = await client.list_resources()
|
||||
supporting = next(r for r in resources if r.name == "utf8-skill/café.md")
|
||||
|
||||
result = await client.read_resource(supporting.uri)
|
||||
assert result[0].text == "UTF8 OK"
|
||||
|
||||
async def test_percent_encoded_name_does_not_collide_with_space(
|
||||
self, tmp_path: Path
|
||||
):
|
||||
"""A filename that already contains a literal '%20' must not be confused
|
||||
with a space-containing filename once both are percent-encoded into
|
||||
resource URIs (#4545)."""
|
||||
skill_dir = tmp_path / "percent-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text("# Skill\n")
|
||||
(skill_dir / "setup guide.md").write_text("SPACE OK")
|
||||
(skill_dir / "setup%20guide.md").write_text("LITERAL PERCENT OK")
|
||||
|
||||
mcp = FastMCP("Test")
|
||||
mcp.add_provider(
|
||||
SkillProvider(skill_path=skill_dir, supporting_files="resources")
|
||||
)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
resources = await client.list_resources()
|
||||
by_name = {r.name: r for r in resources}
|
||||
space_uri = by_name["percent-skill/setup guide.md"].uri
|
||||
literal_uri = by_name["percent-skill/setup%20guide.md"].uri
|
||||
|
||||
assert str(space_uri) != str(literal_uri)
|
||||
|
||||
space_result = await client.read_resource(space_uri)
|
||||
literal_result = await client.read_resource(literal_uri)
|
||||
assert space_result[0].text == "SPACE OK"
|
||||
assert literal_result[0].text == "LITERAL PERCENT OK"
|
||||
|
||||
async def test_skill_resource_meta(self, single_skill_dir: Path):
|
||||
"""SkillResource populates meta with skill name and is_manifest."""
|
||||
provider = SkillProvider(skill_path=single_skill_dir)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue