From b720fc5e387326130c073fd6e4a94014a9c91b60 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Sun, 15 Mar 2026 11:22:13 -0400 Subject: [PATCH] fix: prevent path traversal in skill download (#3493) * fix: prevent path traversal in skill download via malicious skill names Co-authored-by: Claude * fix: resolve skill_dir once and use consistently to prevent overwrite bypass --------- Co-authored-by: Claude --- src/fastmcp/utilities/skills.py | 6 +++++- tests/utilities/test_skills.py | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/fastmcp/utilities/skills.py b/src/fastmcp/utilities/skills.py index a84728fe3..49b13d859 100644 --- a/src/fastmcp/utilities/skills.py +++ b/src/fastmcp/utilities/skills.py @@ -164,7 +164,11 @@ async def download_skill( ``` """ target_dir = Path(target_dir).expanduser().resolve() - skill_dir = target_dir / skill_name + skill_dir = (target_dir / skill_name).resolve() + + # Security: ensure skill_dir stays within target_dir + if not skill_dir.is_relative_to(target_dir): + raise ValueError(f"Skill name {skill_name!r} would escape the target directory") # Check if directory exists if skill_dir.exists() and not overwrite: diff --git a/tests/utilities/test_skills.py b/tests/utilities/test_skills.py index 28a3f4fd1..46170529d 100644 --- a/tests/utilities/test_skills.py +++ b/tests/utilities/test_skills.py @@ -267,3 +267,24 @@ class TestSyncSkills: assert isinstance(path, Path) assert path.exists() assert (path / "SKILL.md").exists() + + +class TestPathTraversal: + @pytest.mark.parametrize( + "malicious_name", + [ + "../escape", + "../../root", + "../../../etc/passwd", + "foo/../../escape", + ], + ) + async def test_malicious_skill_name_raises( + self, skills_server: FastMCP, tmp_path: Path, malicious_name: str + ): + target = tmp_path / "downloaded" + target.mkdir() + + async with Client(skills_server) as client: + with pytest.raises(ValueError, match="would escape the target directory"): + await download_skill(client, malicious_name, target)