fix(security): enforce path boundary checks in skill manager operations

tests/tools/test_skill_manager_tool.py

@@ -5,6 +5,8 @@ from contextlib import contextmanager
 from pathlib import Path
 from unittest.mock import patch
 
+import pytest
+
 from tools.skill_manager_tool import (
     _validate_name,
     _validate_category,
@@ -330,6 +332,25 @@ word word
             result = _patch_skill("nonexistent", "old", "new")
         assert result["success"] is False
 
+    def test_patch_supporting_file_symlink_escape_blocked(self, tmp_path):
+        outside_file = tmp_path / "outside.txt"
+        outside_file.write_text("old text here")
+
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            link = tmp_path / "my-skill" / "references" / "evil.md"
+            link.parent.mkdir(parents=True, exist_ok=True)
+            try:
+                link.symlink_to(outside_file)
+            except OSError:
+                pytest.skip("Symlinks not supported")
+
+            result = _patch_skill("my-skill", "old text", "new text", file_path="references/evil.md")
+
+        assert result["success"] is False
+        assert "boundary" in result["error"].lower()
+        assert outside_file.read_text() == "old text here"
+
 
 class TestDeleteSkill:
     def test_delete_existing(self, tmp_path):
@@ -375,6 +396,25 @@ class TestWriteFile:
             result = _write_file("my-skill", "secret/evil.py", "malicious")
         assert result["success"] is False
 
+    def test_write_symlink_escape_blocked(self, tmp_path):
+        outside_dir = tmp_path / "outside"
+        outside_dir.mkdir()
+
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            link = tmp_path / "my-skill" / "references" / "escape"
+            link.parent.mkdir(parents=True, exist_ok=True)
+            try:
+                link.symlink_to(outside_dir, target_is_directory=True)
+            except OSError:
+                pytest.skip("Symlinks not supported")
+
+            result = _write_file("my-skill", "references/escape/owned.md", "malicious")
+
+        assert result["success"] is False
+        assert "boundary" in result["error"].lower()
+        assert not (outside_dir / "owned.md").exists()
+
 
 class TestRemoveFile:
     def test_remove_existing_file(self, tmp_path):
@@ -391,6 +431,27 @@ class TestRemoveFile:
             result = _remove_file("my-skill", "references/nope.md")
         assert result["success"] is False
 
+    def test_remove_symlink_escape_blocked(self, tmp_path):
+        outside_dir = tmp_path / "outside"
+        outside_dir.mkdir()
+        outside_file = outside_dir / "keep.txt"
+        outside_file.write_text("content")
+
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            link = tmp_path / "my-skill" / "references" / "escape"
+            link.parent.mkdir(parents=True, exist_ok=True)
+            try:
+                link.symlink_to(outside_dir, target_is_directory=True)
+            except OSError:
+                pytest.skip("Symlinks not supported")
+
+            result = _remove_file("my-skill", "references/escape/keep.txt")
+
+        assert result["success"] is False
+        assert "boundary" in result["error"].lower()
+        assert outside_file.exists()
+
 
 # ---------------------------------------------------------------------------
 # skill_manage dispatcher

tools/skill_manager_tool.py

@@ -40,7 +40,7 @@ import shutil
 import tempfile
 from pathlib import Path
 from hermes_constants import get_hermes_home
-from typing import Dict, Any, Optional
+from typing import Dict, Any, Optional, Tuple
 
 logger = logging.getLogger(__name__)
 
@@ -240,6 +240,20 @@ def _validate_file_path(file_path: str) -> Optional[str]:
     return None
 
 
+def _resolve_skill_target(skill_dir: Path, file_path: str) -> Tuple[Optional[Path], Optional[str]]:
+    """Resolve a supporting-file path and ensure it stays within the skill directory."""
+    target = skill_dir / file_path
+    try:
+        resolved = target.resolve(strict=False)
+        skill_dir_resolved = skill_dir.resolve()
+        resolved.relative_to(skill_dir_resolved)
+    except ValueError:
+        return None, "Path escapes skill directory boundary."
+    except OSError as e:
+        return None, f"Invalid file path '{file_path}': {e}"
+    return target, None
+
+
 def _atomic_write_text(file_path: Path, content: str, encoding: str = "utf-8") -> None:
     """
     Atomically write text content to a file.
@@ -394,7 +408,9 @@ def _patch_skill(
         err = _validate_file_path(file_path)
         if err:
             return {"success": False, "error": err}
-        target = skill_dir / file_path
+        target, err = _resolve_skill_target(skill_dir, file_path)
+        if err:
+            return {"success": False, "error": err}
     else:
         # Patching SKILL.md
         target = skill_dir / "SKILL.md"
@@ -500,7 +516,9 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."}
 
-    target = existing["path"] / file_path
+    target, err = _resolve_skill_target(existing["path"], file_path)
+    if err:
+        return {"success": False, "error": err}
     target.parent.mkdir(parents=True, exist_ok=True)
     # Back up for rollback
     original_content = target.read_text(encoding="utf-8") if target.exists() else None
@@ -533,7 +551,9 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
         return {"success": False, "error": f"Skill '{name}' not found."}
     skill_dir = existing["path"]
 
-    target = skill_dir / file_path
+    target, err = _resolve_skill_target(skill_dir, file_path)
+    if err:
+        return {"success": False, "error": err}
     if not target.exists():
         # List what's actually there for the model to see
         available = []