Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 85 additions & 28 deletions src/plugin_system/plugin_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Extracted from PluginManager to improve separation of concerns.
"""

import hashlib
import json
import importlib
import importlib.util
Expand Down Expand Up @@ -138,53 +139,98 @@
self,
plugin_dir: Path,
plugin_id: str,
plugins_dir: Optional[Path] = None,
timeout: int = 300
) -> bool:
"""
Install plugin dependencies from requirements.txt.

Args:
plugin_dir: Plugin directory path
plugin_id: Plugin identifier
plugins_dir: Trusted base plugins directory for path containment check
timeout: Installation timeout in seconds

Returns:
True if dependencies installed or not needed, False on error
"""
plugin_id = os.path.basename(plugin_id or '')
if not plugin_id:
return False
# Resolve and validate plugin_dir before constructing any derived paths

# Resolve to a canonical absolute path (normalises .. and symlinks)
plugin_dir_real = os.path.realpath(str(plugin_dir))

if plugins_dir is not None:
# Reconstruct the plugin path from a trusted base + a sanitised
# directory name. os.path.basename() is CodeQL's recognised
# py/path-injection sanitiser: it strips all directory components
# so the result cannot contain traversal sequences. Joining it
# with the resolved, trusted plugins_dir produces a path that
# CodeQL considers untainted.
plugins_dir_real = os.path.realpath(str(plugins_dir))
safe_dir_name = os.path.basename(plugin_dir_real)
if not safe_dir_name:
self.logger.error("Could not determine plugin directory name for %s", plugin_id)
return False
safe_plugin_dir = os.path.join(plugins_dir_real, safe_dir_name)
if not os.path.isdir(safe_plugin_dir):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
Comment thread
coderabbitai[bot] marked this conversation as resolved.
self.logger.error(
"Plugin directory for %s not found inside plugins dir", plugin_id
)
return False
else:
safe_plugin_dir = plugin_dir_real
if not os.path.isdir(safe_plugin_dir):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
self.logger.error("Plugin directory does not exist: %s", plugin_dir)
return False

requirements_file = os.path.join(safe_plugin_dir, "requirements.txt")
marker_file = os.path.join(safe_plugin_dir, ".dependencies_installed")

if not os.path.isfile(requirements_file):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
return True # No dependencies needed

try:
plugin_dir_resolved = plugin_dir.resolve(strict=True)
except OSError:
self.logger.error("Plugin directory does not exist: %s", plugin_dir)
with open(requirements_file, 'rb') as fh:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
current_hash = hashlib.sha256(fh.read()).hexdigest()
except OSError as e:
self.logger.error("Failed to read requirements.txt for %s: %s", plugin_id, e)
return False
requirements_file = plugin_dir_resolved / "requirements.txt"
if not requirements_file.exists():
return True # No dependencies needed
marker_path = plugin_dir_resolved / ".dependencies_installed"

# Check if already installed
if marker_path.exists():
self.logger.debug("Dependencies already installed for %s", plugin_id)
return True

# Skip if requirements.txt hasn't changed since last install
if os.path.isfile(marker_file):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
try:
with open(marker_file, 'r', encoding='utf-8') as fh:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
stored_hash = fh.read().strip()
except OSError as e:
self.logger.warning(
"Could not read dependency marker for %s (%s), will reinstall dependencies",
plugin_id, e
)
else:
if stored_hash == current_hash:
self.logger.debug("Dependencies already installed for %s (requirements unchanged)", plugin_id)
return True
self.logger.info("Requirements changed for %s, reinstalling dependencies", plugin_id)

try:
self.logger.info("Installing dependencies for plugin %s...", plugin_id)
result = subprocess.run(
[sys.executable, "-m", "pip", "install", "--break-system-packages", "-r", str(requirements_file)],
[sys.executable, "-m", "pip", "install", "--break-system-packages", "-r", requirements_file],
capture_output=True,
text=True,
timeout=timeout,
check=False
)

if result.returncode == 0:
# Mark as installed
marker_path.touch()
# Set proper file permissions after creating marker
ensure_file_permissions(marker_path, get_plugin_file_mode())
try:
with open(marker_file, 'w', encoding='utf-8') as fh:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
fh.write(current_hash)
ensure_file_permissions(Path(marker_file), get_plugin_file_mode())
except OSError as marker_err:
self.logger.debug("Could not write dependency marker for %s: %s", plugin_id, marker_err)
self.logger.info("Dependencies installed successfully for %s", plugin_id)
return True
else:
Expand All @@ -199,8 +245,12 @@
"Assuming they are satisfied: %s",
plugin_id, stderr.strip()
)
marker_path.touch()
ensure_file_permissions(marker_path, get_plugin_file_mode())
try:
with open(marker_file, 'w', encoding='utf-8') as fh:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment thread
ChuckBuilds marked this conversation as resolved.
Dismissed
fh.write(current_hash)
ensure_file_permissions(Path(marker_file), get_plugin_file_mode())
except OSError as marker_err:
self.logger.debug("Could not write dependency marker for %s: %s", plugin_id, marker_err)
return True
self.logger.warning(
"Dependency installation returned non-zero exit code for %s: %s",
Expand Down Expand Up @@ -543,11 +593,12 @@
display_manager: Any,
cache_manager: Any,
plugin_manager: Any,
install_deps: bool = True
install_deps: bool = True,
plugins_dir: Optional[Path] = None,
) -> Tuple[Any, Any]:
"""
Complete plugin loading process.

Args:
plugin_id: Plugin identifier
manifest: Plugin manifest
Expand All @@ -557,16 +608,22 @@
cache_manager: Cache manager instance
plugin_manager: Plugin manager instance
install_deps: Whether to install dependencies

plugins_dir: Trusted base plugins directory forwarded to install_dependencies

Returns:
Tuple of (plugin_instance, module)

Raises:
PluginError: If loading fails
"""
# Install dependencies if needed
if install_deps:
self.install_dependencies(plugin_dir, plugin_id)
if not self.install_dependencies(plugin_dir, plugin_id, plugins_dir=plugins_dir):
raise PluginError(
f"Dependency installation failed for plugin {plugin_id} in {plugin_dir}",
plugin_id=plugin_id,
context={'plugin_dir': str(plugin_dir)},
)

# Load module
entry_point = manifest.get('entry_point', 'manager.py')
Expand Down
3 changes: 2 additions & 1 deletion src/plugin_system/plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ def load_plugin(self, plugin_id: str) -> bool:
display_manager=self.display_manager,
cache_manager=self.cache_manager,
plugin_manager=self,
install_deps=True
install_deps=True,
plugins_dir=self.plugins_dir,
)

# Store module
Expand Down
7 changes: 7 additions & 0 deletions src/plugin_system/store_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from both the official registry and custom GitHub repositories.
"""

import hashlib
import os
import json
import stat
Expand Down Expand Up @@ -1755,6 +1756,12 @@ def _install_dependencies(self, plugin_path: Path) -> bool:
timeout=300
)
self.logger.info(f"Dependencies installed successfully for {plugin_path.name}")
# Write hash marker so plugin_loader skips redundant pip run on next startup
try:
current_hash = hashlib.sha256(requirements_file.read_bytes()).hexdigest()
(plugin_path / ".dependencies_installed").write_text(current_hash, encoding='utf-8')
except OSError as marker_err:
self.logger.debug("Could not write dependency marker for %s: %s", plugin_path.name, marker_err)
return True

except subprocess.CalledProcessError as e:
Expand Down