From 30044c786476cbe121c8dfa9e760795929f45d69 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:34:09 -0400 Subject: [PATCH] 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(). --- .../server/providers/skills/skill_provider.py | 10 ++- .../server/providers/test_skills_provider.py | 90 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/fastmcp_slim/fastmcp/server/providers/skills/skill_provider.py b/fastmcp_slim/fastmcp/server/providers/skills/skill_provider.py index 0550e9042..c08cc5ecb 100644 --- a/fastmcp_slim/fastmcp/server/providers/skills/skill_provider.py +++ b/fastmcp_slim/fastmcp/server/providers/skills/skill_provider.py @@ -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( diff --git a/tests/server/providers/test_skills_provider.py b/tests/server/providers/test_skills_provider.py index b40d60864..051f68fdb 100644 --- a/tests/server/providers/test_skills_provider.py +++ b/tests/server/providers/test_skills_provider.py @@ -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)