fix: prevent path traversal in skill download (#3493)

* fix: prevent path traversal in skill download via malicious skill names

Co-authored-by: Claude <noreply@anthropic.com>

* fix: resolve skill_dir once and use consistently to prevent overwrite bypass

---------

Co-authored-by: Claude <noreply@anthropic.com>

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:

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)