fix: use the system config file (PLUGINS_CONFIG_FILE) for syspath update (Consistent with how the PluginManager works).

Signed-off-by: habeck <habeck@us.ibm.com>

Author: tedhabeck

cpex/framework/isolated/client.py

@@ -17,6 +17,7 @@
 import shutil
 import sys
 import venv
+import yaml
 from pathlib import Path
 
 from typing_extensions import Any, Optional
@@ -26,6 +27,7 @@
 from cpex.framework.errors import PluginError, convert_exception_to_error
 from cpex.framework.hooks.registry import get_hook_registry
 from cpex.framework.isolated.venv_comm import VenvProcessCommunicator
+from cpex.framework.loader.config import ConfigLoader
 from cpex.framework.models import PluginConfig, PluginContext, PluginErrorModel, PluginPayload, PluginResult
 
 logger = logging.getLogger(__name__)
@@ -39,13 +41,16 @@ def __init__(self, config: PluginConfig) -> None:
         super().__init__(config)
         self.implementation = "Python"
         self.comm = None
-        self.script_path: str = config.config["script_path"]
-        path = Path(self.config.config.get("script_path")).resolve()
+        tmp  = os.environ.get("PLUGINS_CONFIG_FILE","plugins/config.yaml")
+        plugin_loader_config = ConfigLoader.load_config(Path(tmp).resolve(), use_jinja=False)
+        self.plugin_dirs = plugin_loader_config.plugin_dirs
+        # use the first plugin dir specified in the plugin configuration file.
+        path = Path(self.plugin_dirs[0]).resolve()
         class_root = self.config.config.get("class_name").split(".")[0]
         cache_root = path / class_root
         self.plugin_path = cache_root
         if not cache_root.exists():
-            raise RuntimeError("plugin script_path does not exist")
+            raise RuntimeError(f"plugin path does not exist: {str(cache_root)}")
         self.cache_dir: Path = cache_root / ".cpex" / "venv_cache"
         self.cache_dir.mkdir(parents=True, exist_ok=True)
 
@@ -269,7 +274,6 @@ def _build_hook_task(self, hook_type: str, payload: PluginPayload, context: Plug
             Task dictionary ready for transmission
         """
         # Cache config lookups
-        script_path = self.config.config["script_path"]
         class_name = self.config.config["class_name"]
         safe_config = self.config.get_safe_config()
 
@@ -279,7 +283,7 @@ def _build_hook_task(self, hook_type: str, payload: PluginPayload, context: Plug
 
         return {
             "task_type": "load_and_run_hook",
-            "script_path": script_path,
+            "plugin_dirs": self.plugin_dirs,
             "class_name": class_name,
             "config": safe_config,
             HOOK_TYPE: hook_type,

cpex/framework/isolated/venv_comm.py

@@ -87,6 +87,7 @@ def start_worker(self, script_path: str) -> None:
                 text=True,
                 bufsize=1,  # Line buffered
                 cwd=os.getcwd(),
+                env={'PLUGINS_CONFIG_FILE': os.environ.get("PLUGINS_CONFIG_FILE", "plugins/config.yaml")}
             )
 
             self.running = True

cpex/framework/isolated/worker.py

@@ -13,11 +13,12 @@
 import importlib.metadata
 import json
 import logging
+import os
 import platform
 import sys
 from pathlib import Path
 from types import ModuleType
-from typing import Type, cast
+from typing import List, Type, cast
 
 from cpex.framework.base import HookRef, Plugin, PluginRef
 from cpex.framework.constants import HOOK_TYPE
@@ -70,11 +71,12 @@ def get_environment_info():
     }
 
 
-def get_proper_config(name, module_path):
+def get_proper_config(name):
     """
     Load a config which has all it's proper decorations
     """
-    plugin_loader_config = ConfigLoader.load_config(Path(f"{module_path}/config.yaml").resolve(), use_jinja=False)
+    plugin_config_file = os.environ.get("PLUGINS_CONFIG_FILE", "plugins/config.yaml")
+    plugin_loader_config = ConfigLoader.load_config(Path(plugin_config_file).resolve(), use_jinja=False)
     plugins: list[dict] = []
     config = None
     if plugin_loader_config.plugins:
@@ -102,21 +104,18 @@ async def process_task(task_data, tp: TaskProcessor):
         # relative path from project root.
         json_config = task_data.get("config")
         config_raw = json.loads(json_config)
-        module_path: str = task_data.get("script_path")
-        
-        # Security: Validate module_path to prevent directory traversal
-        if ".." in module_path or module_path.startswith("/"):
-            raise ValueError(f"Invalid module_path: '{module_path}' - path traversal not allowed")
-        
-        if tp.module_path_hash != tp.compute_hash(module_path) or tp.config_hash != tp.compute_hash(json_config):
-            # pull the resolved plugin path and only add the module path if it has the same root
+        module_paths: List[str] = task_data.get("plugin_dirs")
+        for module_path in module_paths:
             path = Path(module_path).resolve()
             resolved_module_path = str(path)
             if path.exists():
                 sys.path.append(resolved_module_path)
             else:
                 raise RuntimeError(f"plugin module_path '{resolved_module_path}' does not exist.")
-            config = get_proper_config(config_raw.get("name"), module_path)
+
+        if tp.config_hash != tp.compute_hash(json_config):
+            # pull the resolved plugin path and only add the module path if it has the same root
+            config = get_proper_config(config_raw.get("name"))
             hook_type = task_data.get(HOOK_TYPE)
             cls_name: str = task_data.get("class_name")
             mod_name, n_cls_name = parse_class_name(cls_name)

cpex/templates/isolated/{{cookiecutter.plugin_slug}}/config.yaml

@@ -23,8 +23,6 @@ plugins:
       # Plugin config dict passed to the plugin constructor
       class_name: "{{ cookiecutter.plugin_slug }}.plugin.{{ class_name }}"
       requirements_file: "requirements.txt"
-      # essentially the plugin folder hosting the plugin
-      script_path: "{{ cookiecutter.plugin_slug }}"
 
 # Plugin directories to scan
 plugin_dirs:

tests/unit/cpex/framework/isolated/test_client.py

@@ -29,7 +29,6 @@ class TestIsolatedVenvPlugin:
     def mock_config(self, tmp_path):
         """Create a mock plugin configuration."""
         venv_path = tmp_path / ".venv"
-        script_path = "tests/unit/cpex/fixtures/plugins/isolated"
         requirements_file = tmp_path / "test_plugin" / "requirements.txt"
 
         config_dict = {
@@ -43,7 +42,6 @@ def mock_config(self, tmp_path):
                 "class_name": "test_plugin.TestPlugin",
                 "venv_path": venv_path,
                 "requirements_file": requirements_file,
-                "script_path": script_path
             }
         }
 
@@ -63,11 +61,10 @@ def plugin_context(self):
         )
         return plugin_context
 
-    def test_init(self, plugin, mock_config):
+    def test_init(self, plugin):
         """Test plugin initialization."""
         assert plugin.name == "test_plugin"
         assert plugin.implementation == "Python"
-        assert plugin.script_path == mock_config.config["script_path"]
         assert plugin.comm is None
 
     @pytest.mark.asyncio

tests/unit/cpex/framework/isolated/test_integration.py

@@ -67,10 +67,7 @@ async def test_plugin_manager_with_isolated_plugin(
         # Create manager
         manager = PluginManager(integration_config_path)
 
-        # This will fail because the config path doesn't exist in the test environment
-        # but we can test the structure
-        with pytest.raises(RuntimeError):
-            await manager.initialize()
+        await manager.initialize()
 
     @pytest.mark.asyncio
     @patch("cpex.framework.isolated.client.VenvProcessCommunicator")

tests/unit/cpex/framework/isolated/test_worker.py

@@ -22,6 +22,14 @@
 class TestWorkerFunctions:
     """Test suite for worker.py functions."""
 
+    @pytest.fixture
+    def mock_plugin_dirs(self, tmp_path):
+        """ensure that the plugins directory exists"""
+        plugin_dirs = tmp_path / "plugins"
+        tmp = Path(plugin_dirs)
+        tmp.mkdir(parents=True, exist_ok=True)
+        return [str(plugin_dirs.resolve())]
+
     def test_get_environment_info(self):
         """Test getting environment information."""
         info = get_environment_info()
@@ -48,7 +56,7 @@ def test_get_proper_config_found(self, mock_load_config):
         mock_config.plugins = [mock_plugin]
         mock_load_config.return_value = mock_config
 
-        result = get_proper_config("test_plugin", "plugins")
+        result = get_proper_config("test_plugin")
 
         assert result is not None
         assert result.name == "test_plugin"
@@ -63,7 +71,7 @@ def test_get_proper_config_not_found(self, mock_load_config):
         mock_config.plugins = [mock_plugin]
         mock_load_config.return_value = mock_config
 
-        result = get_proper_config("test_plugin", "plugins")
+        result = get_proper_config("test_plugin")
 
         assert result is None
 
@@ -74,7 +82,7 @@ def test_get_proper_config_no_plugins(self, mock_load_config):
         mock_config.plugins = None
         mock_load_config.return_value = mock_config
 
-        result = get_proper_config("test_plugin", "plugins")
+        result = get_proper_config("test_plugin")
 
         assert result is None
 
@@ -95,7 +103,7 @@ async def test_process_task_info(self):
     @patch("cpex.framework.isolated.worker.get_proper_config")
     @patch("cpex.framework.isolated.worker.importlib.import_module")
     @patch("cpex.framework.isolated.worker.PluginExecutor")
-    async def test_process_task_load_and_run_hook_success(self, mock_executor_class, mock_import, mock_get_config):
+    async def test_process_task_load_and_run_hook_success(self, mock_executor_class, mock_import, mock_get_config, mock_plugin_dirs):
         """Test processing load_and_run_hook task successfully."""
         # Setup mock config
         mock_config = MagicMock()
@@ -127,7 +135,7 @@ async def test_process_task_load_and_run_hook_success(self, mock_executor_class,
         task_data = {
             "task_type": "load_and_run_hook",
             "config": json.dumps(config_dict),
-            "script_path": "plugins",
+            "plugin_dirs": mock_plugin_dirs,
             "class_name": "test_plugin.TestPlugin",
             "hook_type": "tool_pre_invoke",
             "payload": {"name": "test_tool", "args": {}},
@@ -150,7 +158,6 @@ async def test_process_task_load_and_run_hook_no_config(self, mock_get_config):
         task_data = {
             "task_type": "load_and_run_hook",
             "config": json.dumps(config_dict),
-            "script_path": "plugins",
             "class_name": "test_plugin.TestPlugin",
             "hook_type": "tool_pre_invoke",
             "payload": {},
@@ -164,7 +171,7 @@ async def test_process_task_load_and_run_hook_no_config(self, mock_get_config):
     @pytest.mark.asyncio
     @patch("cpex.framework.isolated.worker.get_proper_config")
     @patch("cpex.framework.isolated.worker.importlib.import_module")
-    async def test_process_task_load_and_run_hook_import_error(self, mock_import, mock_get_config):
+    async def test_process_task_load_and_run_hook_import_error(self, mock_import, mock_get_config, mock_plugin_dirs):
         """Test processing load_and_run_hook task with import error."""
         mock_config = MagicMock()
         mock_get_config.return_value = mock_config
@@ -175,8 +182,8 @@ async def test_process_task_load_and_run_hook_import_error(self, mock_import, mo
         task_data = {
             "task_type": "load_and_run_hook",
             "config": json.dumps(config_dict),
-            "script_path": "plugins",
             "class_name": "test_plugin.TestPlugin",
+            "plugin_dirs": mock_plugin_dirs,
             "hook_type": "tool_pre_invoke",
             "payload": {},
             "context": {"state": {}, "global_context": {}, "metadata": {}},
@@ -189,7 +196,7 @@ async def test_process_task_load_and_run_hook_import_error(self, mock_import, mo
     @patch("cpex.framework.isolated.worker.get_proper_config")
     @patch("cpex.framework.isolated.worker.importlib.import_module")
     @patch("cpex.framework.isolated.worker.PluginExecutor")
-    async def test_process_task_with_different_hook_types(self, mock_executor_class, mock_import, mock_get_config):
+    async def test_process_task_with_different_hook_types(self, mock_executor_class, mock_import, mock_get_config, mock_plugin_dirs):
         """Test processing tasks with different hook types."""
         # Setup mocks
         mock_config = MagicMock()
@@ -222,7 +229,7 @@ async def test_process_task_with_different_hook_types(self, mock_executor_class,
             task_data = {
                 "task_type": "load_and_run_hook",
                 "config": json.dumps(config_dict),
-                "script_path": "plugins",
+                "plugin_dirs": mock_plugin_dirs,
                 "class_name": "test_plugin.TestPlugin",
                 "hook_type": hook_type,
                 "payload": {},
@@ -244,7 +251,7 @@ async def test_process_task_unknown_task_type(self):
     @patch("cpex.framework.isolated.worker.get_proper_config")
     @patch("cpex.framework.isolated.worker.importlib.import_module")
     @patch("cpex.framework.isolated.worker.PluginExecutor")
-    async def test_process_task_with_metadata(self, mock_executor_class, mock_import, mock_get_config):
+    async def test_process_task_with_metadata(self, mock_executor_class, mock_import, mock_get_config, mock_plugin_dirs):
         """Test processing task with metadata in context."""
         mock_config = MagicMock()
         mock_get_config.return_value = mock_config
@@ -273,8 +280,8 @@ async def test_process_task_with_metadata(self, mock_executor_class, mock_import
         task_data = {
             "task_type": "load_and_run_hook",
             "config": json.dumps(config_dict),
-            "script_path": "plugins",
             "class_name": "test_plugin.TestPlugin",
+            "plugin_dirs": mock_plugin_dirs,
             "hook_type": "tool_pre_invoke",
             "payload": {"name": "test_tool"},
             "context": {
@@ -402,7 +409,6 @@ async def test_main_with_load_and_run_hook_task(self, mock_process_task, mock_pr
         task_data = {
             "task_type": "load_and_run_hook",
             "config": json.dumps(config_dict),
-            "script_path": "plugins",
             "class_name": "test_plugin.TestPlugin",
             "hook_type": "tool_pre_invoke",
             "payload": {"name": "test_tool"},