🛡️ Sentinel: [CRITICAL] Fix Path Traversal in Kokoro Engine

Sanitized user-provided filenames across `resolve_voice_path`, `mix_voices`, `load_preset`, and `load_fx_preset` in `kokoro_engine.py` using `os.path.basename()` to prevent directory traversal attacks (e.g. `../../../etc/passwd`). Added `test_security.py` using mocked dependencies to verify the fix. Documented findings in `.jules/sentinel.md`.

Author: CoffeeMethod

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2024-05-24 - [Path Traversal in Parsed Multi-speaker Syntax]
+**Vulnerability:** User-controlled filenames generated from parsed multi-speaker text (e.g. `[speaker_name]:`) or direct inputs to engine mix/load endpoints lacked sanitization in `kokoro_engine.py`, resulting in Path Traversal vulnerabilities when fetching presets, voices, and fx files using `os.path.join()`.
+**Learning:** Due to the complex text parsing mechanism, what initially seems like clean preset names parsed out of input strings might contain relative traversal directories, making the system unexpectedly vulnerable.
+**Prevention:** Always use `os.path.basename()` or equivalent file name sanitization before passing parsed components or user inputs to `os.path.join()`.

kokoro_engine.py

@@ -106,6 +106,9 @@ def resolve_voice_path(self, voice_name):
         Returns the absolute path if it's a custom voice, 
         otherwise returns the name as-is (for standard voices).
         """
+        # Sanitize to prevent path traversal
+        voice_name = os.path.basename(voice_name)
+
         # Check if it's a custom voice file
         custom_path = os.path.join(CUSTOM_VOICES_DIR, f"{voice_name}.pt")
         if os.path.exists(custom_path):
@@ -318,7 +321,8 @@ def _mix():
                     mixed = t1 * (1.0 - ratio) + t2 * ratio
 
                 # Save
-                out_path = os.path.join(CUSTOM_VOICES_DIR, f"{new_name}.pt")
+                new_name_clean = os.path.basename(new_name)
+                out_path = os.path.join(CUSTOM_VOICES_DIR, f"{new_name_clean}.pt")
                 torch.save(mixed, out_path)
                 return True, out_path, mixed
             except Exception as e:
@@ -479,6 +483,8 @@ def parse_multispeaker_text(self, text):
 
     def load_preset(self, name):
         """Loads a preset from the presets directory."""
+        # Sanitize name to prevent path traversal
+        name = os.path.basename(name)
         preset_path = os.path.join("presets", f"{name}.json")
         if os.path.exists(preset_path):
             try:
@@ -490,6 +496,8 @@ def load_preset(self, name):
 
     def load_fx_preset(self, name):
         """Loads an FX preset from the presets/fx directory."""
+        # Sanitize name to prevent path traversal
+        name = os.path.basename(name)
         fx_path = os.path.join("presets", "fx", f"{name}.json")
         if os.path.exists(fx_path):
             try:

test_security.py

@@ -0,0 +1,61 @@
+import sys
+import unittest
+from unittest.mock import MagicMock
+import os
+
+# Mock dependencies
+sys.modules['soundfile'] = MagicMock()
+sys.modules['torch'] = MagicMock()
+sys.modules['numpy'] = MagicMock()
+sys.modules['scipy'] = MagicMock()
+sys.modules['scipy.signal'] = MagicMock()
+sys.modules['pedalboard'] = MagicMock()
+sys.modules['pedalboard.io'] = MagicMock()
+sys.modules['pypdf'] = MagicMock()
+sys.modules['ebooklib'] = MagicMock()
+sys.modules['ebooklib.epub'] = MagicMock()
+sys.modules['bs4'] = MagicMock()
+sys.modules['winsound'] = MagicMock()
+sys.modules['kokoro'] = MagicMock()
+
+# Now we can import the engine
+from kokoro_engine import KokoroEngine
+
+class TestSecurity(unittest.TestCase):
+    def setUp(self):
+        self.engine = KokoroEngine()
+
+    def test_path_traversal_resolve_voice_path(self):
+        payload = "../../../../../etc/passwd"
+        path = self.engine.resolve_voice_path(payload)
+        self.assertNotIn("etc", path)
+        self.assertNotIn("..", path)
+        self.assertFalse("passwd" in path and "/" in path)
+
+    def test_path_traversal_load_preset(self):
+        payload = "../../../../../etc/passwd"
+        # If it doesn't fail parsing or trying to open /etc/passwd, we expect it to look in presets/
+        # Just checking if the constructed path in load_preset would have been bad is tricky
+        # since load_preset catches exception and returns None.
+        # We can mock os.path.exists and open if needed, or rely on the fix in the engine.
+        # Let's mock open to see what it tries to open
+        with unittest.mock.patch('builtins.open', unittest.mock.mock_open(read_data='{}')) as m:
+            with unittest.mock.patch('os.path.exists', return_value=True):
+                self.engine.load_preset(payload)
+                m.assert_called_once()
+                args, kwargs = m.call_args
+                self.assertNotIn("..", args[0])
+                self.assertNotIn("etc", args[0])
+
+    def test_path_traversal_load_fx_preset(self):
+        payload = "../../../../../etc/passwd"
+        with unittest.mock.patch('builtins.open', unittest.mock.mock_open(read_data='{}')) as m:
+            with unittest.mock.patch('os.path.exists', return_value=True):
+                self.engine.load_fx_preset(payload)
+                m.assert_called_once()
+                args, kwargs = m.call_args
+                self.assertNotIn("..", args[0])
+                self.assertNotIn("etc", args[0])
+
+if __name__ == '__main__':
+    unittest.main()