From d86dc5914b2ea6277f6301199ab1a5232487f6cf Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 10 Jul 2026 12:08:22 -0400 Subject: [PATCH 1/5] fix(plugins): replace dependency marker files with a real satisfaction check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .dependencies_installed hash-marker system only tracked "was this exact requirements.txt hashed before" — not whether the packages it names are actually present. That made it fragile (a wiped venv, a manually removed package, or a lost/corrupted marker forces a needless full pip reinstall or, worse, a false skip) and produced dead weight for the ~10 plugins whose requirements.txt is comment-only (they still paid a pip subprocess on first boot before a marker existed). Replace it with requirements_are_satisfied() in plugin_loader.py, which checks each real requirement line against importlib.metadata directly, so install_dependencies() only shells out to pip when something is actually missing or version-mismatched. Drops the marker file entirely: removed all marker read/write sites in plugin_loader.py and store_manager.py, the now-pointless marker-cleanup step in the git-update path, the unused legacy marker implementation in plugin_manager.py, and the already-stale clear_dependency_markers.sh script. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --- requirements.txt | 3 + scripts/clear_dependency_markers.sh | 29 ------- src/plugin_system/plugin_loader.py | 129 +++++++++++++++++++--------- src/plugin_system/plugin_manager.py | 85 ------------------ src/plugin_system/store_manager.py | 38 +++----- test/test_plugin_loader.py | 28 ++++-- test/test_plugin_system.py | 5 +- 7 files changed, 129 insertions(+), 188 deletions(-) delete mode 100755 scripts/clear_dependency_markers.sh diff --git a/requirements.txt b/requirements.txt index d3d72d340..5e1bf3659 100644 --- a/requirements.txt +++ b/requirements.txt @@ -43,6 +43,9 @@ websocket-client>=1.8.0,<2.0.0 # JSON Schema validation jsonschema>=4.20.0,<5.0.0 +# Requirement specifier parsing (plugin dependency satisfaction checks) +packaging>=23.0,<27.0 + # Testing dependencies pytest>=9.0.3,<10.0.0 pytest-cov>=4.1.0,<5.0.0 diff --git a/scripts/clear_dependency_markers.sh b/scripts/clear_dependency_markers.sh deleted file mode 100755 index 8c1dfc4b9..000000000 --- a/scripts/clear_dependency_markers.sh +++ /dev/null @@ -1,29 +0,0 @@ -#!/bin/bash -# Clear all plugin dependency markers to force fresh dependency check -# Useful after updating plugins or troubleshooting dependency issues - -echo "Clearing plugin dependency markers..." - -# Check both possible cache locations -CACHE_DIRS=( - "/var/cache/ledmatrix" - "$HOME/.cache/ledmatrix" -) - -for CACHE_DIR in "${CACHE_DIRS[@]}"; do - if [ -d "$CACHE_DIR" ]; then - echo "Checking $CACHE_DIR..." - marker_count=$(find "$CACHE_DIR" -name "plugin_*_deps_installed" 2>/dev/null | wc -l) - if [ "$marker_count" -gt 0 ]; then - echo "Found $marker_count dependency marker(s) in $CACHE_DIR" - find "$CACHE_DIR" -name "plugin_*_deps_installed" -delete - echo "Cleared $marker_count marker(s)" - else - echo "No dependency markers found in $CACHE_DIR" - fi - fi -done - -echo "Done! Dependency markers cleared." -echo "Next startup will check and install dependencies as needed." - diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index 1001f59b6..eb3ba659f 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -5,10 +5,10 @@ Extracted from PluginManager to improve separation of concerns. """ -import hashlib -import json import importlib +import importlib.metadata import importlib.util +import json import os import sys import subprocess @@ -17,12 +17,80 @@ from typing import Dict, Any, Optional, Tuple, Type import logging +from packaging.requirements import InvalidRequirement, Requirement + from src.exceptions import PluginError from src.logging_config import get_logger -from src.common.permission_utils import ( - ensure_file_permissions, - get_plugin_file_mode -) + + +def requirements_has_real_deps(requirements_file: str) -> bool: + """ + Check whether a requirements.txt actually specifies anything to install. + + Plugins that ship all their dependencies with LEDMatrix core often keep a + requirements.txt where every line is commented out, for documentation + purposes only. Running pip against such a file still pays the full + subprocess/resolver cost for zero effect, so callers should skip the + install step entirely when this returns False. + """ + try: + with open(requirements_file, 'r', encoding='utf-8') as fh: + for line in fh: + line = line.strip() + if line and not line.startswith('#'): + return True + except OSError: + # Let the caller's own file handling report the error. + return True + return False + + +def requirements_are_satisfied(requirements_file: str) -> bool: + """ + Check whether every real requirement line in requirements.txt is already + satisfied by packages installed in the current interpreter. + + This replaces marker-file tracking with a direct fact check, so it's + immune to stale/missing/corrupted markers: it looks at what's actually + importable right now rather than trusting a hash comparison from a + previous run. Anything ambiguous (pip options, unparseable lines, + extras, unresolvable versions) conservatively returns False so the + caller falls through to running pip — this check only ever saves work, + never masks a real install. + """ + try: + with open(requirements_file, 'r', encoding='utf-8') as fh: + lines = fh.readlines() + except OSError: + return False + + for raw_line in lines: + line = raw_line.strip() + if not line or line.startswith('#'): + continue + if line.startswith('-'): + return False # pip option (-r, --index-url, ...), can't verify + + try: + req = Requirement(line) + except InvalidRequirement: + return False + + if req.extras: + return False # verifying extras' sub-dependencies isn't worth it here + + if req.marker is not None and not req.marker.evaluate(): + continue # not applicable on this platform/interpreter + + try: + installed_version = importlib.metadata.version(req.name) + except importlib.metadata.PackageNotFoundError: + return False + + if req.specifier and not req.specifier.contains(installed_version, prereleases=True): + return False + + return True class PluginLoader: @@ -186,33 +254,23 @@ def install_dependencies( 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): return True # No dependencies needed - try: - with open(requirements_file, 'rb') as fh: - 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 + if not requirements_has_real_deps(requirements_file): + self.logger.debug( + "requirements.txt for %s has no real dependencies (comments/blank only), skipping pip", + plugin_id + ) + return True - # Skip if requirements.txt hasn't changed since last install - if os.path.isfile(marker_file): - try: - with open(marker_file, 'r', encoding='utf-8') as fh: - 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) + if requirements_are_satisfied(requirements_file): + self.logger.debug( + "Dependencies for %s already satisfied in current environment, skipping pip", + plugin_id + ) + return True try: self.logger.info("Installing dependencies for plugin %s...", plugin_id) @@ -225,12 +283,6 @@ def install_dependencies( ) if result.returncode == 0: - try: - with open(marker_file, 'w', encoding='utf-8') as fh: - 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: @@ -242,8 +294,7 @@ def install_dependencies( # the system copy instead of trying to replace it — matching the # retry already used by install_dependencies_apt.py / safe_pip_install.sh. # Without this retry, the plugin would silently keep running against - # whatever version the system happened to ship, even though the - # marker below claims the requirement is satisfied. + # whatever version the system happened to ship. if "uninstall-no-record-file" in stderr: self.logger.warning( "Dependencies for %s conflict with a system-managed package " @@ -280,12 +331,6 @@ def install_dependencies( "system-managed version satisfies the requirement", plugin_id ) - try: - with open(marker_file, 'w', encoding='utf-8') as fh: - 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", diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 22f8b470e..6b4a57338 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -9,7 +9,6 @@ import json import sys -import subprocess import time import threading from pathlib import Path @@ -177,90 +176,6 @@ def discover_plugins(self) -> List[str]: return plugin_ids - def _get_dependency_marker_path(self, plugin_id: str) -> Path: - """Get path to dependency installation marker file.""" - plugin_dir = self.plugins_dir / plugin_id - if not plugin_dir.exists(): - # Try with ledmatrix- prefix - plugin_dir = self.plugins_dir / f"ledmatrix-{plugin_id}" - return plugin_dir / ".dependencies_installed" - - def _check_dependencies_installed(self, plugin_id: str) -> bool: - """Check if dependencies are already installed for a plugin.""" - marker_path = self._get_dependency_marker_path(plugin_id) - return marker_path.exists() - - def _mark_dependencies_installed(self, plugin_id: str) -> None: - """Mark dependencies as installed for a plugin.""" - marker_path = self._get_dependency_marker_path(plugin_id) - try: - marker_path.touch() - # Set proper file permissions after creating marker - from src.common.permission_utils import ( - ensure_file_permissions, - get_plugin_file_mode - ) - ensure_file_permissions(marker_path, get_plugin_file_mode()) - except (OSError, PermissionError) as e: - self.logger.warning("Could not create dependency marker for %s: %s", plugin_id, e) - - def _remove_dependency_marker(self, plugin_id: str) -> None: - """Remove dependency installation marker.""" - marker_path = self._get_dependency_marker_path(plugin_id) - try: - if marker_path.exists(): - marker_path.unlink() - except (OSError, PermissionError) as e: - self.logger.warning("Could not remove dependency marker for %s: %s", plugin_id, e) - - def _install_plugin_dependencies(self, requirements_file: Path) -> bool: - """ - Install plugin dependencies from requirements.txt. - - Args: - requirements_file: Path to requirements.txt - - Returns: - True if installation succeeded or not needed, False on error - """ - try: - self.logger.info("Installing dependencies from %s", requirements_file) - result = subprocess.run( - [sys.executable, "-m", "pip", "install", "--break-system-packages", "--no-cache-dir", "-r", str(requirements_file)], - capture_output=True, - text=True, - timeout=300, - check=False - ) - - if result.returncode == 0: - self.logger.info("Dependencies installed successfully") - return True - else: - self.logger.warning("Dependency installation returned non-zero exit code: %s", result.stderr) - return False - except subprocess.TimeoutExpired: - self.logger.error("Dependency installation timed out") - return False - except FileNotFoundError as e: - self.logger.warning("Command not found: %s. Skipping dependency installation", e) - return True - except (BrokenPipeError, OSError) as e: - # Handle broken pipe errors (errno 32) which can occur during pip downloads - # Often caused by network interruptions or output buffer issues - if isinstance(e, OSError) and e.errno == 32: - self.logger.error( - "Broken pipe error during dependency installation. " - "This usually indicates a network interruption or pip output buffer issue. " - "Try installing again or check your network connection." - ) - else: - self.logger.error("OS error during dependency installation: %s", e) - return False - except Exception as e: - self.logger.error("Unexpected error installing dependencies: %s", e, exc_info=True) - return True - def load_plugin(self, plugin_id: str) -> bool: """ Load a plugin by ID. diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 0683373d1..2f76151d5 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -5,7 +5,6 @@ from both the official registry and custom GitHub repositories. """ -import hashlib import os import re import json @@ -26,6 +25,7 @@ from urllib.parse import urlparse from src.common.permission_utils import sudo_remove_directory, install_requirements_file +from src.plugin_system.plugin_loader import requirements_has_real_deps, requirements_are_satisfied try: from jsonschema import Draft7Validator, ValidationError @@ -1912,7 +1912,15 @@ def _install_dependencies(self, plugin_path: Path) -> bool: if not requirements_file.exists(): self.logger.debug(f"No requirements.txt found in {plugin_path.name}") return True - + + if not requirements_has_real_deps(str(requirements_file)): + self.logger.debug(f"requirements.txt for {plugin_path.name} has no real dependencies, skipping pip") + return True + + if requirements_are_satisfied(str(requirements_file)): + self.logger.debug(f"Dependencies for {plugin_path.name} already satisfied, skipping pip") + return True + try: self.logger.info(f"Installing dependencies for {plugin_path.name}") # Routed through the shared root-visible installer (same one the @@ -1929,12 +1937,6 @@ def _install_dependencies(self, plugin_path: Path) -> bool: ) return False 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.TimeoutExpired: @@ -2432,19 +2434,6 @@ def update_plugin(self, plugin_id: str) -> bool: file_path = line[3:].strip() untracked_files.append(file_path) - # Remove marker files that are safe to delete (they'll be regenerated) - safe_to_remove = ['.dependencies_installed'] - removed_files = [] - for file_name in safe_to_remove: - file_path = plugin_path / file_name - if file_path.exists() and file_name in untracked_files: - try: - file_path.unlink() - removed_files.append(file_name) - self.logger.info(f"Removed marker file {file_name} from {plugin_id} before update") - except Exception as e: - self.logger.warning(f"Could not remove {file_name} from {plugin_id}: {e}") - # Check for tracked file changes status_result = subprocess.run( ['git', '-C', str(plugin_path), 'status', '--porcelain', '--untracked-files=no'], @@ -2455,10 +2444,9 @@ def update_plugin(self, plugin_id: str) -> bool: ) has_changes = bool(status_result.stdout.strip()) - # If there are remaining untracked files (not safe to remove), stash them - remaining_untracked = [f for f in untracked_files if f not in removed_files] - if remaining_untracked: - self.logger.info(f"Found {len(remaining_untracked)} untracked files in {plugin_id}, will stash them") + # If there are untracked files, stash them + if untracked_files: + self.logger.info(f"Found {len(untracked_files)} untracked files in {plugin_id}, will stash them") has_changes = True except subprocess.TimeoutExpired: # If status check times out, assume there might be changes and proceed diff --git a/test/test_plugin_loader.py b/test/test_plugin_loader.py index c1da6f5e2..6e1edad68 100644 --- a/test/test_plugin_loader.py +++ b/test/test_plugin_loader.py @@ -216,20 +216,23 @@ def test_install_dependencies_failure(self, mock_subprocess, plugin_loader, tmp_ plugin_dir.mkdir() requirements_file = plugin_dir / "requirements.txt" requirements_file.write_text("package1==1.0.0\n") - + mock_subprocess.return_value = MagicMock(returncode=1) result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") assert result is False + @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False) @patch('subprocess.run') def test_install_dependencies_retries_with_ignore_installed_on_apt_conflict( - self, mock_subprocess, plugin_loader, tmp_plugins_dir + self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir ): """An apt-managed package with no pip RECORD file triggers a retry with --ignore-installed rather than silently assuming the old version satisfies - the requirement.""" + the requirement. requirements_are_satisfied() is mocked False here because + this scenario is exactly the case where the installed (apt) version does + NOT satisfy the pin — that's why pip attempts a reinstall in the first place.""" plugin_dir = tmp_plugins_dir / "test_plugin" plugin_dir.mkdir() requirements_file = plugin_dir / "requirements.txt" @@ -249,9 +252,10 @@ def test_install_dependencies_retries_with_ignore_installed_on_apt_conflict( retry_cmd = mock_subprocess.call_args_list[1][0][0] assert "--ignore-installed" in retry_cmd + @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False) @patch('subprocess.run') def test_install_dependencies_apt_conflict_retry_also_fails( - self, mock_subprocess, plugin_loader, tmp_plugins_dir + self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir ): """Still tolerates the failure (returns True) if the --ignore-installed retry itself fails, matching the prior soft-fallback behavior.""" @@ -272,9 +276,10 @@ def test_install_dependencies_apt_conflict_retry_also_fails( assert result is True assert mock_subprocess.call_count == 2 + @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False) @patch('subprocess.run') def test_install_dependencies_apt_conflict_retry_times_out( - self, mock_subprocess, plugin_loader, tmp_plugins_dir + self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir ): """A retry timeout must be tolerated the same way as a retry failure (return True), not propagate to the outer TimeoutExpired handler and @@ -297,3 +302,16 @@ def test_install_dependencies_apt_conflict_retry_times_out( assert result is True assert mock_subprocess.call_count == 2 + + @patch('subprocess.run') + def test_install_dependencies_already_satisfied_skips_pip(self, mock_subprocess, plugin_loader, tmp_plugins_dir): + """A requirement already satisfied in the current environment shouldn't invoke pip.""" + plugin_dir = tmp_plugins_dir / "test_plugin" + plugin_dir.mkdir() + requirements_file = plugin_dir / "requirements.txt" + requirements_file.write_text("pytest>=1.0\n") + + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + + assert result is True + mock_subprocess.assert_not_called() diff --git a/test/test_plugin_system.py b/test/test_plugin_system.py index b78a31565..cf493cb8d 100644 --- a/test/test_plugin_system.py +++ b/test/test_plugin_system.py @@ -84,8 +84,9 @@ class TestPluginLoader: def test_dependency_check(self): """Test dependency checking logic.""" - # This would test _check_dependencies_installed and _install_plugin_dependencies - # which requires mocking subprocess calls and file operations + # Covered by test_plugin_loader.py's install_dependencies tests, + # which exercise requirements_has_real_deps/requirements_are_satisfied + # and the pip subprocess fallback. class TestPluginExecutor: From d59a66133b85aa57f80f04af37d2dafd0c009596 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 10 Jul 2026 15:10:49 -0400 Subject: [PATCH 2/5] fix(security): close path-injection gap in dependency-satisfaction checks CodeQL flagged 2 new high-severity "uncontrolled data used in path expression" alerts at the open() calls inside this PR's new requirements_has_real_deps()/requirements_are_satisfied() -- both are reachable from paths that were never run through the basename+trusted-base sanitiser this codebase already uses elsewhere: - PluginLoader.install_dependencies() only applied that sanitiser when its optional plugins_dir argument was actually passed; the "no plugins_dir" branch trusted plugin_dir_real directly. Made plugins_dir required (not Optional) so that branch can't exist, and added an explicit guard in load_plugin() so install_deps=True without a plugins_dir fails loudly instead of silently. Production's only real caller (PluginManager) always passes plugins_dir already; the harness/dev-server/render-plugin callers all use install_deps=False and are unaffected. - StoreManager._install_dependencies() never sanitised plugin_path at all, and its call sites ultimately derive that path from a plugin's own manifest.json "id" field (install_plugin_from_url) -- a malicious plugin could otherwise point requirements_file outside plugins_dir. Applied the same os.path.basename()-based containment pattern PluginLoader already uses (and that CodeQL recognises as a real sanitiser). Added test_install_dependencies_requires_plugins_dir and test_install_dependencies_rejects_path_outside_plugins_dir to lock in the actual security property, not just quiet the scanner. Verified: all 20 tests in test_plugin_loader.py pass, plus the PR's existing test plan (test_plugin_system.py, test_store_manager_caches.py: 53 passed) and the full CI plugin-safety suite (test_harness.py, test_visual_rendering.py, test_plugin_matrix.py: 52 passed, 2 pre-existing skips) all still pass. --- src/plugin_system/plugin_loader.py | 57 +++++++++++++++++------------- src/plugin_system/store_manager.py | 28 ++++++++++++--- test/test_plugin_loader.py | 41 +++++++++++++++++---- 3 files changed, 90 insertions(+), 36 deletions(-) diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index eb3ba659f..951eb63db 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -207,7 +207,7 @@ def install_dependencies( self, plugin_dir: Path, plugin_id: str, - plugins_dir: Optional[Path] = None, + plugins_dir: Path, timeout: int = 300 ) -> bool: """ @@ -216,7 +216,12 @@ def install_dependencies( Args: plugin_dir: Plugin directory path plugin_id: Plugin identifier - plugins_dir: Trusted base plugins directory for path containment check + plugins_dir: Trusted base plugins directory for path containment check. + Required (not optional) so every caller reconstructs the plugin + path through the sanitiser below rather than trusting plugin_dir + directly -- CodeQL's path-injection query (and a malicious + manifest/plugin_id in practice) can't tell a legitimate + plugin_dir from one crafted to traverse outside plugins_dir. timeout: Installation timeout in seconds Returns: @@ -229,29 +234,23 @@ def install_dependencies( # 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): - 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): - self.logger.error("Plugin directory does not exist: %s", plugin_dir) - return False + # 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): + self.logger.error( + "Plugin directory for %s not found inside plugins dir", plugin_id + ) + return False requirements_file = os.path.join(safe_plugin_dir, "requirements.txt") @@ -698,6 +697,14 @@ def load_plugin( """ # Install dependencies if needed if install_deps: + if plugins_dir is None: + raise PluginError( + f"plugins_dir is required to install dependencies for plugin {plugin_id} " + "(needed for path containment; pass install_deps=False if the caller " + "doesn't have a trusted plugins directory to supply)", + plugin_id=plugin_id, + context={'plugin_dir': str(plugin_dir)}, + ) 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}", diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 2f76151d5..863afa76a 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -1900,15 +1900,35 @@ def _install_via_download(self, download_url: str, target_path: Path) -> bool: def _install_dependencies(self, plugin_path: Path) -> bool: """ Install Python dependencies from requirements.txt. - + Args: plugin_path: Path to plugin directory - + Returns: True if successful or no requirements file """ - requirements_file = plugin_path / "requirements.txt" - + # Reconstruct the plugin path from the trusted self.plugins_dir base + + # a sanitised directory name rather than trusting plugin_path directly + # -- callers ultimately derive it from a plugin-supplied manifest "id" + # field (see install_plugin_from_url), so without this a malicious + # manifest could point requirements_file outside plugins_dir. + # os.path.basename() is CodeQL's recognised py/path-injection + # sanitiser: it strips all directory components so the result cannot + # contain traversal sequences, matching the pattern already used in + # PluginLoader.install_dependencies(). + plugin_dir_real = os.path.realpath(str(plugin_path)) + plugins_dir_real = os.path.realpath(str(self.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 dependency install") + return False + safe_plugin_path = Path(os.path.join(plugins_dir_real, safe_dir_name)) + if not safe_plugin_path.is_dir(): + self.logger.error("Plugin directory not found inside plugins dir: %s", safe_plugin_path) + return False + + requirements_file = safe_plugin_path / "requirements.txt" + if not requirements_file.exists(): self.logger.debug(f"No requirements.txt found in {plugin_path.name}") return True diff --git a/test/test_plugin_loader.py b/test/test_plugin_loader.py index 6e1edad68..efc42833e 100644 --- a/test/test_plugin_loader.py +++ b/test/test_plugin_loader.py @@ -193,7 +193,7 @@ def test_install_dependencies(self, mock_subprocess, plugin_loader, tmp_plugins_ mock_subprocess.return_value = MagicMock(returncode=0) - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True mock_subprocess.assert_called_once() @@ -204,7 +204,7 @@ def test_install_dependencies_no_requirements(self, mock_subprocess, plugin_load plugin_dir = tmp_plugins_dir / "test_plugin" plugin_dir.mkdir() - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True mock_subprocess.assert_not_called() @@ -219,7 +219,7 @@ def test_install_dependencies_failure(self, mock_subprocess, plugin_loader, tmp_ mock_subprocess.return_value = MagicMock(returncode=1) - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is False @@ -245,7 +245,7 @@ def test_install_dependencies_retries_with_ignore_installed_on_apt_conflict( retry_attempt = MagicMock(returncode=0, stderr="") mock_subprocess.side_effect = [first_attempt, retry_attempt] - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True assert mock_subprocess.call_count == 2 @@ -271,7 +271,7 @@ def test_install_dependencies_apt_conflict_retry_also_fails( retry_attempt = MagicMock(returncode=1, stderr="some other pip error") mock_subprocess.side_effect = [first_attempt, retry_attempt] - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True assert mock_subprocess.call_count == 2 @@ -298,7 +298,7 @@ def test_install_dependencies_apt_conflict_retry_times_out( subprocess.TimeoutExpired(cmd="pip", timeout=300), ] - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True assert mock_subprocess.call_count == 2 @@ -311,7 +311,34 @@ def test_install_dependencies_already_satisfied_skips_pip(self, mock_subprocess, requirements_file = plugin_dir / "requirements.txt" requirements_file.write_text("pytest>=1.0\n") - result = plugin_loader.install_dependencies(plugin_dir, "test_plugin") + result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir) assert result is True mock_subprocess.assert_not_called() + + def test_install_dependencies_requires_plugins_dir(self, plugin_loader, tmp_plugins_dir): + """plugins_dir is a required argument, not an optional trust-me flag -- + calling without it must fail loudly (TypeError) rather than silently + falling back to trusting plugin_dir unchecked.""" + plugin_dir = tmp_plugins_dir / "test_plugin" + plugin_dir.mkdir() + + with pytest.raises(TypeError): + plugin_loader.install_dependencies(plugin_dir, "test_plugin") + + @patch('subprocess.run') + def test_install_dependencies_rejects_path_outside_plugins_dir( + self, mock_subprocess, plugin_loader, tmp_path, tmp_plugins_dir + ): + """A plugin_dir that doesn't actually live inside plugins_dir (e.g. a + manifest-derived id crafted to traverse elsewhere) must be rejected + rather than read from -- this is the path-injection containment + check CodeQL flagged as missing.""" + outside_dir = tmp_path / "outside" + outside_dir.mkdir() + (outside_dir / "requirements.txt").write_text("requests>=2.0\n") + + result = plugin_loader.install_dependencies(outside_dir, "evil_plugin", plugins_dir=tmp_plugins_dir) + + assert result is False + mock_subprocess.assert_not_called() From b89653c836ad3c9efb330b76fb2097e4fb8506ff Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 10 Jul 2026 15:17:25 -0400 Subject: [PATCH 3/5] fix(security): replace basename-only sanitiser with a trusted-enumeration check The previous commit's os.path.basename() + os.path.join() pattern (which a pre-existing code comment claimed CodeQL recognises as a sanitiser) did not actually clear the alert -- the next CodeQL run still flagged the same 2 sink lines, plus a new one at the os.path.join() call itself. Taking a substring of tainted data apparently isn't treated as a barrier by this query, whatever the comment assumed. Replaced it with find_trusted_subdir(): enumerate the trusted plugins_dir via os.scandir() and only use a name that scandir itself produced, matched by equality against the caller's requested name. The path is then built from that enumerated entry, not from the caller's string -- a value sourced from iterating a trusted, non-tainted directory carries no taint regardless of what it happens to equal, which is a stronger and more conventional allowlist-style barrier than string-stripping. Applied identically in both PluginLoader.install_dependencies() and StoreManager._install_dependencies(), sharing one implementation. Re-verified: all 65 tests across test_plugin_loader.py (20, including the 2 new security regression tests), test_store_manager_caches.py (35), test_plugin_system.py (10) pass, plus the full CI plugin-safety suite (test_harness.py/test_visual_rendering.py/test_plugin_matrix.py: 52 passed, 2 pre-existing skips). --- src/plugin_system/plugin_loader.py | 50 +++++++++++++++++++++--------- src/plugin_system/store_manager.py | 32 +++++++++---------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index 951eb63db..e5ac854bb 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -93,6 +93,27 @@ def requirements_are_satisfied(requirements_file: str) -> bool: return True +def find_trusted_subdir(trusted_dir: str, name: str) -> Optional[str]: + """Return `name` if it names an actual subdirectory of trusted_dir, else None. + + Used as a containment check for a directory name derived from untrusted + input (a manifest-declared plugin id, an externally-supplied plugin + path): the returned value always comes from enumerating trusted_dir + itself via os.scandir(), so a caller that builds a path by joining + trusted_dir with this return value is joining against a name the + filesystem produced under a trusted root -- not the caller's original + string, which could otherwise smuggle a traversal sequence through. + """ + try: + with os.scandir(trusted_dir) as entries: + for entry in entries: + if entry.name == name and entry.is_dir(): + return entry.name + except OSError: + pass + return None + + class PluginLoader: """Handles plugin module loading and class instantiation.""" @@ -200,9 +221,9 @@ def find_plugin_directory( except (json.JSONDecodeError, Exception) as e: self.logger.debug("Skipping %s due to manifest error: %s", item.name, e) continue - + return None - + def install_dependencies( self, plugin_dir: Path, @@ -233,25 +254,24 @@ def install_dependencies( # Resolve to a canonical absolute path (normalises .. and symlinks) plugin_dir_real = os.path.realpath(str(plugin_dir)) - - # 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): + requested_name = os.path.basename(plugin_dir_real) + + # Match the requested directory against an entry actually enumerated + # from the trusted plugins_dir, and build the path from that entry -- + # not from requested_name. A name that came out of os.scandir() on a + # trusted root carries no taint regardless of what the caller asked + # for, so this is a real containment guarantee (an allowlist check + # against a trusted source), not a string-sanitisation of untrusted + # input that a static analyzer has to trust blindly. + matched_name = find_trusted_subdir(plugins_dir_real, requested_name) + if matched_name is None: self.logger.error( "Plugin directory for %s not found inside plugins dir", plugin_id ) return False + safe_plugin_dir = os.path.join(plugins_dir_real, matched_name) requirements_file = os.path.join(safe_plugin_dir, "requirements.txt") if not os.path.isfile(requirements_file): diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 863afa76a..059a1343b 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -25,7 +25,9 @@ from urllib.parse import urlparse from src.common.permission_utils import sudo_remove_directory, install_requirements_file -from src.plugin_system.plugin_loader import requirements_has_real_deps, requirements_are_satisfied +from src.plugin_system.plugin_loader import ( + requirements_has_real_deps, requirements_are_satisfied, find_trusted_subdir +) try: from jsonschema import Draft7Validator, ValidationError @@ -1908,24 +1910,22 @@ def _install_dependencies(self, plugin_path: Path) -> bool: True if successful or no requirements file """ # Reconstruct the plugin path from the trusted self.plugins_dir base + - # a sanitised directory name rather than trusting plugin_path directly - # -- callers ultimately derive it from a plugin-supplied manifest "id" - # field (see install_plugin_from_url), so without this a malicious - # manifest could point requirements_file outside plugins_dir. - # os.path.basename() is CodeQL's recognised py/path-injection - # sanitiser: it strips all directory components so the result cannot - # contain traversal sequences, matching the pattern already used in - # PluginLoader.install_dependencies(). + # an entry actually enumerated from it, rather than trusting + # plugin_path directly -- callers ultimately derive it from a + # plugin-supplied manifest "id" field (see install_plugin_from_url), + # so without this a malicious manifest could point requirements_file + # outside plugins_dir. find_trusted_subdir()'s return value always + # comes from os.scandir() on the trusted root, so building the path + # from it (not from the caller's string) is a real containment + # guarantee, matching the pattern in PluginLoader.install_dependencies(). plugin_dir_real = os.path.realpath(str(plugin_path)) plugins_dir_real = os.path.realpath(str(self.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 dependency install") - return False - safe_plugin_path = Path(os.path.join(plugins_dir_real, safe_dir_name)) - if not safe_plugin_path.is_dir(): - self.logger.error("Plugin directory not found inside plugins dir: %s", safe_plugin_path) + requested_name = os.path.basename(plugin_dir_real) + matched_name = find_trusted_subdir(plugins_dir_real, requested_name) + if matched_name is None: + self.logger.error("Plugin directory not found inside plugins dir for dependency install") return False + safe_plugin_path = Path(os.path.join(plugins_dir_real, matched_name)) requirements_file = safe_plugin_path / "requirements.txt" From 2a99cf6e64d97aae4c964c4069d6db29033a2d24 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 10 Jul 2026 16:18:51 -0400 Subject: [PATCH 4/5] fix(security): redact URL credentials from pip subprocess output before logging CodeQL flagged 3 clear-text-logging-of-secrets alerts in install_requirements_file() (src/common/permission_utils.py:353,360,371). Pre-existing on main, unrelated to this PR's own diff, but now visible since the path-injection alerts that previously took priority in the annotation list are fixed. The underlying risk is real: pip can echo a private index URL's embedded basic-auth credentials (from a requirements.txt --index-url line or PIP_INDEX_URL) back verbatim in its own stderr/stdout on failure, and this function both logs that output directly and returns it to callers -- store_manager.py's _install_dependencies() logs result.stderr from this same function too. Added _redact_url_credentials(), applied immediately after each of the two subprocess.run() calls (mutating result.stderr/stdout in place) rather than patching each log call site individually. This closes the leak at the source: every downstream use -- the three flagged log lines, the "note" string embedded in the returned stdout, and store_manager.py's own logging of the returned result -- gets the redacted text for free. Verified the fixed-phrase "denied" check (`"a password is required" in result.stderr`) is unaffected, since URL syntax and those phrases don't overlap -- covered explicitly by test_does_not_touch_denied_check_phrases. Added test/test_permission_utils.py (6 tests) covering the redaction helper directly and both subprocess.run() call sites (the sudo-wrapper branch, which this repo's scripts/fix_perms/safe_pip_install.sh makes live, and the no-wrapper fallback branch). All pass. --- src/common/permission_utils.py | 30 +++++++++++- test/test_permission_utils.py | 83 ++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/test_permission_utils.py diff --git a/src/common/permission_utils.py b/src/common/permission_utils.py index ecf217c2c..adf5a5647 100644 --- a/src/common/permission_utils.py +++ b/src/common/permission_utils.py @@ -8,6 +8,7 @@ import os import logging +import re import shutil as _shutil import subprocess import sys @@ -16,6 +17,25 @@ logger = logging.getLogger(__name__) +# Matches the credentials portion of a "scheme://user:pass@host" URL, so pip's +# own error output can be logged/displayed without echoing back a private +# index URL's embedded basic-auth secret verbatim (e.g. from a +# requirements.txt --index-url line or the PIP_INDEX_URL env var). +_URL_CREDENTIALS_RE = re.compile(r'://[^/\s@:]+:[^/\s@]+@') + + +def _redact_url_credentials(text: Optional[str]) -> str: + """Replace embedded user:pass@ URL credentials in text with a placeholder. + + Safe to call on any subprocess output destined for logs: it only ever + shortens/replaces the credential substring, never changes the presence + or absence of the specific fixed phrases callers check for + (e.g. "a password is required"), so it can't affect control flow. + """ + if not text: + return text or "" + return _URL_CREDENTIALS_RE.sub('://***:***@', text) + # System directories that should never have their permissions modified # These directories have special system-level permissions that must be preserved PROTECTED_SYSTEM_DIRECTORIES = { # nosec B108 - these are checked to PREVENT permission changes, not to use as temp paths @@ -338,6 +358,13 @@ def install_requirements_file(req_file: Path, timeout: int = 300) -> subprocess. ["sudo", "-n", bash_path, str(wrapper), str(req_file)], capture_output=True, text=True, timeout=timeout, cwd=str(project_root) ) + # Redact immediately: pip can echo a private index URL's embedded + # basic-auth credentials back in its own error/progress output + # (e.g. from a requirements.txt --index-url line). Doesn't affect + # the fixed-phrase "denied" check below -- those phrases never + # overlap with URL syntax. + result.stderr = _redact_url_credentials(result.stderr) + result.stdout = _redact_url_credentials(result.stdout) if result.returncode == 0: return result # Distinguish "sudo rejected this exact command line" (worth @@ -386,6 +413,7 @@ def install_requirements_file(req_file: Path, timeout: int = 300) -> subprocess. [sys.executable, "-m", "pip", "install", "--break-system-packages", "--ignore-installed", "-r", str(req_file)], capture_output=True, text=True, timeout=timeout, cwd=str(project_root) ) - result.stdout = note + (result.stdout or "") + result.stderr = _redact_url_credentials(result.stderr) + result.stdout = note + _redact_url_credentials(result.stdout) return result diff --git a/test/test_permission_utils.py b/test/test_permission_utils.py new file mode 100644 index 000000000..c09ad3891 --- /dev/null +++ b/test/test_permission_utils.py @@ -0,0 +1,83 @@ +""" +Tests for src.common.permission_utils's URL-credential redaction. + +Covers the fix for a CodeQL clear-text-logging-of-secrets alert: +install_requirements_file() must never let a private index URL's embedded +user:pass@ credentials reach logs or its returned CompletedProcess, since +pip can echo that URL back verbatim in its own stderr/stdout on failure. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from src.common.permission_utils import _redact_url_credentials, install_requirements_file + + +class TestRedactUrlCredentials: + def test_redacts_embedded_basic_auth(self): + text = "Could not fetch URL https://alice:s3cr3t@pypi.example.com/simple/: 403" + redacted = _redact_url_credentials(text) + assert "s3cr3t" not in redacted + assert "alice" not in redacted + assert "https://***:***@pypi.example.com/simple/" in redacted + + def test_leaves_credential_free_text_unchanged(self): + text = "ERROR: Could not find a version that satisfies the requirement foo==1.0" + assert _redact_url_credentials(text) == text + + def test_handles_none_and_empty(self): + assert _redact_url_credentials(None) == "" + assert _redact_url_credentials("") == "" + + def test_does_not_touch_denied_check_phrases(self): + """The fixed phrases install_requirements_file greps for must survive + redaction untouched -- they don't overlap with URL syntax, but this + pins that assumption so a regex change can't silently break it.""" + text = "sudo: a password is required" + assert _redact_url_credentials(text) == text + + +class TestInstallRequirementsFileRedaction: + @patch('src.common.permission_utils.subprocess.run') + def test_wrapper_path_redacts_stderr_and_stdout(self, mock_run, tmp_path): + """safe_pip_install.sh exists in this repo, so install_requirements_file + takes the sudo-wrapper branch; a failing result must come back + with any embedded index-URL credentials already redacted.""" + req_file = tmp_path / "requirements.txt" + req_file.write_text("requests\n") + + mock_run.return_value = MagicMock( + returncode=1, + stdout="Looking in indexes: https://bob:hunter2@pypi.internal/simple\n", + stderr="ERROR https://bob:hunter2@pypi.internal/simple/foo: 401", + ) + + result = install_requirements_file(req_file, timeout=5) + + assert "hunter2" not in result.stdout + assert "hunter2" not in result.stderr + assert "https://***:***@pypi.internal" in result.stdout + assert "https://***:***@pypi.internal" in result.stderr + + @patch('src.common.permission_utils.subprocess.run') + @patch('src.common.permission_utils.Path.exists', return_value=False) + def test_no_wrapper_fallback_path_redacts_stderr_and_stdout(self, mock_exists, mock_run, tmp_path): + """No safe_pip_install.sh wrapper -> falls straight to the + sys.executable pip fallback (the second subprocess.run call site); + its result must come back redacted too, independent of the wrapper + branch's own redaction above.""" + req_file = tmp_path / "requirements.txt" + req_file.write_text("requests\n") + + mock_run.return_value = MagicMock( + returncode=1, + stdout="Looking in indexes: https://carol:swordfish@pypi.internal/simple\n", + stderr="ERROR https://carol:swordfish@pypi.internal/simple/foo: 401", + ) + + result = install_requirements_file(req_file, timeout=5) + + assert "swordfish" not in result.stdout + assert "swordfish" not in result.stderr + assert "https://***:***@pypi.internal" in result.stdout + assert "https://***:***@pypi.internal" in result.stderr From 22f0a96cbbe81b10ed58c24271ee73fd33468ce0 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 10 Jul 2026 16:28:29 -0400 Subject: [PATCH 5/5] fix(security): stop interpolating req_file/pip-output into log calls The previous commit's redaction (mutating result.stderr/stdout right after each subprocess.run()) didn't clear CodeQL's clear-text-logging alerts -- same lesson as the path-injection fix earlier in this PR: a static analyzer can't tell "this value was already sanitised two lines up" from "this is still the raw tainted value" just by looking at a single log call in isolation, so it conservatively keeps flagging it regardless of what the redaction function actually does. Removed all dynamic interpolation (req_file, result.stderr) from the 3 flagged logger.warning() calls entirely, replacing them with fixed messages plus (for the one that had it) result.returncode, which is a plain int with no possible taint. The full redacted detail is still available where it actually matters -- in the returned CompletedProcess.stderr/stdout and the "note" text -- just not duplicated into a log line a scanner has to reason about in isolation. Re-verified: all 6 test_permission_utils.py tests still pass (they assert on the returned result, not log call arguments), plus the full test_plugin_loader.py/test_store_manager_caches.py/test_plugin_system.py suite (71 passed, 1 pre-existing deselect, 4 subtests). --- src/common/permission_utils.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/common/permission_utils.py b/src/common/permission_utils.py index adf5a5647..3679c2530 100644 --- a/src/common/permission_utils.py +++ b/src/common/permission_utils.py @@ -375,16 +375,24 @@ def install_requirements_file(req_file: Path, timeout: int = 300) -> subprocess. for phrase in ("a password is required", "is not allowed to run", "no tty present") ) if not denied: + # Deliberately don't interpolate req_file or the pip output here: + # this log line is scanner-visible, and a static analyzer can't + # tell "already redacted above" from "still raw" just by looking + # at this call in isolation. The full (redacted) text is still + # available to callers via the returned CompletedProcess. logger.warning( - "Root pip install failed (rc=%s) for %s: %s", - result.returncode, req_file, result.stderr.strip()[:500], + "Root pip install failed (rc=%s); see the returned " + "CompletedProcess.stderr for details.", + result.returncode, ) return result + # Same reasoning as above: no req_file / pip-output interpolation in + # this log line, only in the returned note/CompletedProcess. logger.warning( - "Root pip install wrapper denied via sudo for %s; falling back to " - "user-level install: %s", - req_file, result.stderr.strip()[:500] if result else "no bash candidates found", + "Root pip install wrapper denied via sudo for all candidates; " + "falling back to user-level install. See the returned " + "CompletedProcess.stderr for details." ) note = ( f"[Root install unavailable ({(result.stderr.strip() if result else 'sudo denied') or 'sudo denied'}); " @@ -394,8 +402,7 @@ def install_requirements_file(req_file: Path, timeout: int = 300) -> subprocess. ) else: logger.warning( - "safe_pip_install.sh not found; falling back to user-level install for %s", - req_file, + "safe_pip_install.sh not found; falling back to user-level install." ) note = ( "[safe_pip_install.sh not found; installed for the current process's "