From 3e203de125aaf4608d91defc94060adcad300fae Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Sun, 29 Mar 2026 20:08:22 -0700
Subject: [PATCH] fix(skills): block category path traversal in skill manager
 (#3844)

Validate category names in _create_skill() before using them as
filesystem path segments. Previously, categories like '../escape' or
'/tmp/pwned' could write skill files outside ~/.hermes/skills/.

Adds _validate_category() that rejects slashes, backslashes, absolute
paths, and non-alphanumeric characters (reuses existing VALID_NAME_RE).

Tests: 5 new tests for traversal, absolute paths, and valid categories.

Salvaged from PR #1939 by Gutslabs.
---
 tests/tools/test_skill_manager_tool.py | 40 ++++++++++++++++++++++++++
 tools/skill_manager_tool.py            | 29 +++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/tests/tools/test_skill_manager_tool.py b/tests/tools/test_skill_manager_tool.py
index bd992ec3d..06a2f88ae 100644
--- a/tests/tools/test_skill_manager_tool.py
+++ b/tests/tools/test_skill_manager_tool.py
@@ -6,6 +6,7 @@ from unittest.mock import patch
 
 from tools.skill_manager_tool import (
     _validate_name,
+    _validate_category,
     _validate_frontmatter,
     _validate_file_path,
     _find_skill,
@@ -82,6 +83,22 @@ class TestValidateName:
         assert "Invalid skill name 'skill@name'" in err
 
 
+class TestValidateCategory:
+    def test_valid_categories(self):
+        assert _validate_category(None) is None
+        assert _validate_category("") is None
+        assert _validate_category("devops") is None
+        assert _validate_category("mlops-v2") is None
+
+    def test_path_traversal_rejected(self):
+        err = _validate_category("../escape")
+        assert "Invalid category '../escape'" in err
+
+    def test_absolute_path_rejected(self):
+        err = _validate_category("/tmp/escape")
+        assert "Invalid category '/tmp/escape'" in err
+
+
 # ---------------------------------------------------------------------------
 # _validate_frontmatter
 # ---------------------------------------------------------------------------
@@ -191,6 +208,29 @@ class TestCreateSkill:
             result = _create_skill("my-skill", "no frontmatter here")
         assert result["success"] is False
 
+    def test_create_rejects_category_traversal(self, tmp_path):
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+
+        with patch("tools.skill_manager_tool.SKILLS_DIR", skills_dir):
+            result = _create_skill("my-skill", VALID_SKILL_CONTENT, category="../escape")
+
+        assert result["success"] is False
+        assert "Invalid category '../escape'" in result["error"]
+        assert not (tmp_path / "escape").exists()
+
+    def test_create_rejects_absolute_category(self, tmp_path):
+        skills_dir = tmp_path / "skills"
+        skills_dir.mkdir()
+        outside = tmp_path / "outside"
+
+        with patch("tools.skill_manager_tool.SKILLS_DIR", skills_dir):
+            result = _create_skill("my-skill", VALID_SKILL_CONTENT, category=str(outside))
+
+        assert result["success"] is False
+        assert f"Invalid category '{outside}'" in result["error"]
+        assert not (outside / "my-skill" / "SKILL.md").exists()
+
 
 class TestEditSkill:
     def test_edit_existing_skill(self, tmp_path):
diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py
index 3c8619496..8507a6d13 100644
--- a/tools/skill_manager_tool.py
+++ b/tools/skill_manager_tool.py
@@ -113,6 +113,31 @@ def _validate_name(name: str) -> Optional[str]:
     return None
 
 
+def _validate_category(category: Optional[str]) -> Optional[str]:
+    """Validate an optional category name used as a single directory segment."""
+    if category is None:
+        return None
+    if not isinstance(category, str):
+        return "Category must be a string."
+
+    category = category.strip()
+    if not category:
+        return None
+    if "/" in category or "\\" in category:
+        return (
+            f"Invalid category '{category}'. Use lowercase letters, numbers, "
+            "hyphens, dots, and underscores. Categories must be a single directory name."
+        )
+    if len(category) > MAX_NAME_LENGTH:
+        return f"Category exceeds {MAX_NAME_LENGTH} characters."
+    if not VALID_NAME_RE.match(category):
+        return (
+            f"Invalid category '{category}'. Use lowercase letters, numbers, "
+            "hyphens, dots, and underscores. Categories must be a single directory name."
+        )
+    return None
+
+
 def _validate_frontmatter(content: str) -> Optional[str]:
     """
     Validate that SKILL.md content has proper frontmatter with required fields.
@@ -241,6 +266,10 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An
     if err:
         return {"success": False, "error": err}
 
+    err = _validate_category(category)
+    if err:
+        return {"success": False, "error": err}
+
     # Validate content
     err = _validate_frontmatter(content)
     if err: