feat: plugin venv isolation

[tedhabeck]

Summary

Closes: #5

Changes

This branch represents a significant architectural enhancement enabling plugins to run in isolated Python environments, improving security, dependency management, and plugin compatibility.

Key Enhancements

1. Plugin Isolation via Virtual Environment (Primary Feature)

   New Core Modules:

   • cpex/framework/isolated/client.py - Client for managing isolated plugin execution
   • cpex/framework/isolated/worker.py - Worker process for executing plugins in isolation
   • cpex/framework/isolated/venv_comm.py - Communication layer between host and isolated environment

2. Venv Cache Support
   Caching mechanism for virtual environments to improve performance
   Reduces overhead of repeated venv creation

3. Serialization Support
   Enhanced serialization capabilities for plugin data exchange
   Supports communication between isolated environments

4. Configuration Support
   New fixture: isolated_plugin.yaml for testing isolated plugin configurations
   Updated loader to support isolated plugin mode

Checks

• make lint passes
• make test passes
• CHANGELOG updated (if user-facing)

Notes (optional)

Performance Optimization Complete ✓

Restructured the isolated venv plugin system to use a long-running worker process instead of forking a new subprocess for each invocation.

Performance Improvements

Before (forking new process each time):

prompt_pre_fetch: 0.147ms avg
prompt_post_fetch: 0.155ms avg
tool_pre_invoke: 0.143ms avg
tool_post_invoke: 0.149ms avg
Average: ~0.148ms per invocation

After (long-running worker process):

prompt_pre_fetch: 0.045ms avg
prompt_post_fetch: 0.047ms avg
tool_pre_invoke: 0.043ms avg
tool_post_invoke: 0.044ms avg
Average: ~0.045ms per invocation
Performance Gain: 3.3x faster (70% reduction in latency)

Example Plugin directory structure with venv configured:

[araujof]

Nice work, @tedhabeck !

Below are a few things I think we should discuss and change before merging:

Security

• Arbitrary code loading via sys.path + importlib
  The worker appends user-controlled plugin_dirs to sys.path then dynamically imports class_name via importlib.import_module. If plugin_dirs or class_name originate from untrusted config, this is a remote code execution vector.

Recommendation: Validate plugin_dirs entries against an allowlist (e.g., must be under a known project root). Validate class_name against a pattern or registry.

• subprocess.check_call with user-controlled requirements file

subprocess.check_call([self.python_executable, "-m", "pip", "install", "-r", requirements_file])

The requirements_file path comes from plugin config. A malicious requirements file can install arbitrary packages (supply chain attack) or reference local paths. pip install can execute setup.py which runs arbitrary code.

Recommendation: Validate requirements file path is within the plugin directory.

• No input size limits on stdio communication
  Both the reader thread and worker main() call readline() without any size limit. A malicious or buggy worker could send an arbitrarily large line, causing OOM.

Recommendation: Add a maximum line length check before parsing.

• Worker process inherits CWD
  The worker runs in os.getcwd(). If the host CWD contains sensitive files, the plugin code in the worker has filesystem access to them.

• No authentication on the stdio channel
  Any process that can write to the worker's stdin can send commands. This is inherent to the subprocess model but worth noting if the threat model evolves.

Design

• IsolatedVenvPlugin.__init__ reads config file
  The constructor calls ConfigLoader.load_config() to discover plugin_dirs. This couples plugin instantiation to filesystem config and makes unit testing harder. The plugin dirs should be passed in via PluginConfig or injected.

• Hardcoded PLUGINS_CONFIG_FILE env var fallback
  Both client.py and worker.py default to "plugins/config.yaml". This is duplicated and fragile. Use PluginSettings to define access and defaults to environment variables.

• to_json() method on PluginConfig manually excludes validator names

methods_to_exclude = {"_migrate_legacy_modes", "check_url_or_script_filled", ...}

Adding a new validator requires updating this set. model_dump(mode="json") already excludes methods; the filter is unnecessary since Pydantic v2 doesn't serialize validators.

Bugs

• sys.path pollution in worker
  Every call to process_task appends to sys.path without checking for duplicates. Over many invocations this grows unboundedly.

for module_path in module_paths:
    path = Path(module_path).resolve()
    sys.path.append(resolved_module_path)  # appended every time

• module_path variable leak in TaskProcessor.initialize

tp.initialize(..., module_path=module_path)

module_path here is the loop variable from the for module_path in module_paths loop, so it's always the last directory. This may not be the intended one.

• Error response in worker uses stale task_data

except json.JSONDecodeError as e:
    error_response = {
        "request_id": task_data.get("request_id", "unknown") if "task_data" in locals() else "unknown",
    }

If JSON parsing fails, task_data is from the previous iteration (or undefined). The "task_data" in locals() check is unreliable since the variable persists across loop iterations.

Nits

• print() statements in create_venv should be logger.info().

• Mixed orjson and json in venv_comm.py -- pick one.

• get_environment_info() in worker.py uses deprecated importlib.metadata.entry_points() pattern and slices to [:10] arbitrarily.

Tests

• No tests for the actual subprocess lifecycle (start/send/stop without mocks)
• No tests for concurrent send_task calls (the request-id routing is untested under concurrency)
• No negative tests for sys.path pollution or the module_path variable leak
• PluginPackageInfo and PluginVersionRegistry models have no dedicated tests in this PR

[araujof]

Nice work addressing our reviews, @tedhabeck !

What's been resolved

Addressed security and bug items:

• arbitrary code loading
• requirements file traversal
• unbounded stdio reads
• sys.path pollution
• module_path variable leak
• stale task_data reference

The to_json() design smell was also cleaned up, and the PluginPackageInfo/PluginVersionRegistry models now have dedicated test coverage.

Acceptable to defer

• Hardcoded plugins/config.yaml fallback: The worker dependency was removed, but venv_comm.py:492 still passes os.environ.get("PLUGINS_CONFIG_FILE", "plugins/config.yaml") to the subprocess. This should use PluginSettings for the default rather than a
  hardcoded string.
• Deprecated entry_points() pattern: get_environment_info() still uses the deprecated calling convention and an arbitrary [:10] slice. Minor but easy to fix.
• Worker inherits CWD: Low risk given the allowlist validation now in place, but worth a follow-up if the threat model tightens.
• Concurrency and negative tests: The request-id routing and sys.path dedup logic are now correct, but remain undertested under concurrent load. A follow-up issue for integration/stress tests would be appropriate.

Addressed