Merge pull request #1207 from Kiln-AI/leonard/kil-509-fix-skill-tool-should-validate-reference-path-is-file

fix: error when Skill ref or asset path points to a dir

Author: leonardmq

libs/core/kiln_ai/datamodel/skill.py

@@ -56,6 +56,8 @@ def skill_md_raw(self) -> str:
         md_path = self.skill_md_path()
         if not md_path.exists():
             raise FileNotFoundError(f"SKILL.md not found at {md_path}")
+        if md_path.is_dir():
+            raise FileNotFoundError(f"SKILL.md path is a folder, not a file: {md_path}")
         return md_path.read_text(encoding="utf-8")
 
     def body(self) -> str:
@@ -77,11 +79,11 @@ def assets_dir(self) -> Path:
         return self.path.parent / "assets"
 
     def read_reference(self, relative_path: str) -> str:
-        """Read a reference file. Raises ValueError for path traversal or non-text, FileNotFoundError if missing."""
+        """Read a reference file. Raises ValueError for path traversal, non-text, or if the path is a folder, FileNotFoundError if missing."""
         return self._read_resource(self.references_dir(), relative_path)
 
     def read_asset(self, relative_path: str) -> str:
-        """Read an asset file. Raises ValueError for path traversal or non-text, FileNotFoundError if missing."""
+        """Read an asset file. Raises ValueError for path traversal, non-text, or if the path is a folder, FileNotFoundError if missing."""
         return self._read_resource(self.assets_dir(), relative_path)
 
     def _read_resource(self, base_dir: Path, relative_path: str) -> str:
@@ -96,6 +98,9 @@ def _read_resource(self, base_dir: Path, relative_path: str) -> str:
         except ValueError:
             raise ValueError("Path traversal is not allowed") from None
 
+        if resolved.is_dir():
+            raise ValueError(f"Path is a folder, not a file: {relative_path}")
+
         try:
             return resolved.read_text(encoding="utf-8")
         except FileNotFoundError:

libs/core/kiln_ai/datamodel/test_skill.py

@@ -4,10 +4,7 @@
 from pydantic import ValidationError
 
 from kiln_ai.datamodel.project import Project
-from kiln_ai.datamodel.skill import (
-    Skill,
-    _parse_skill_md_body,
-)
+from kiln_ai.datamodel.skill import Skill, _parse_skill_md_body
 
 
 @pytest.fixture
@@ -109,6 +106,14 @@ def test_body_raises_without_skill_md(mock_project):
         skill.body()
 
 
+def test_skill_md_path_is_directory(mock_project):
+    skill = make_skill(parent=mock_project)
+    skill.save_to_file()
+    skill.skill_md_path().mkdir()
+    with pytest.raises(FileNotFoundError, match=r"SKILL\.md path is a folder"):
+        skill.skill_md_raw()
+
+
 def test_save_skill_md_empty_body_rejected(mock_project):
     skill = make_skill(parent=mock_project)
     skill.save_to_file()
@@ -409,6 +414,12 @@ def test_binary_file_rejected(self, mock_project):
         with pytest.raises(ValueError, match="not a readable text file"):
             skill.read_reference("image.png")
 
+    def test_read_reference_path_is_directory(self, mock_project):
+        skill = save_skill_with_body(mock_project)
+        (skill.references_dir() / "subdir").mkdir()
+        with pytest.raises(ValueError, match="folder, not a file"):
+            skill.read_reference("subdir")
+
     def test_references_dir_requires_saved_skill(self):
         skill = make_skill()
         with pytest.raises(ValueError, match="Skill must be saved"):
@@ -446,6 +457,12 @@ def test_asset_binary_file_rejected(self, mock_project):
         with pytest.raises(ValueError, match="not a readable text file"):
             skill.read_asset("photo.jpg")
 
+    def test_read_asset_path_is_directory(self, mock_project):
+        skill = save_skill_with_body(mock_project)
+        (skill.assets_dir() / "data_dir").mkdir()
+        with pytest.raises(ValueError, match="folder, not a file"):
+            skill.read_asset("data_dir")
+
     def test_assets_dir_requires_saved_skill(self):
         skill = make_skill()
         with pytest.raises(ValueError, match="Skill must be saved"):