fix: initialise wifi-status throttle state in __init__

Codacy (pylint access-member-before-definition) on #403: the throttled
early-return read _wifi_status_last_result relying on the non-local
invariant that the first call always passes the throttle window and
assigns it. Correct at runtime, but fragile — initialise both throttle
fields in the constructor and drop the getattr fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam

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

scripts/clear_dependency_markers.sh

@@ -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."
-

src/common/permission_utils.py

@@ -8,6 +8,7 @@ files that need to be accessible by both root service and web user.
 
 import os
 import logging
+import re
 import shutil as _shutil
 import subprocess
 import sys
@@ -16,6 +17,25 @@ from typing import Optional
 
 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
@@ -348,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'}); "
@@ -367,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 "
@@ -386,6 +420,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
 

src/display_controller.py

@@ -199,6 +199,10 @@ class DisplayController:
         self.wifi_status_file = WIFI_STATUS_FILE
         self.wifi_status_active = False
         self.wifi_status_expires_at: Optional[float] = None
+        # _check_wifi_status_message throttle state (checked at frame rate,
+        # stat'd at most once per second)
+        self._wifi_status_check_ts = 0.0
+        self._wifi_status_last_result: Optional[Dict[str, Any]] = None
 
         # Plugin display() signature cache — must be initialised before the plugin
         # loading loop below so the .pop() invalidation at load time is always safe.
@@ -1635,7 +1639,7 @@ class DisplayController:
                     self._sleep_with_plugin_updates(60)
                     continue
                 
-                logger.info(f"Display active, processing mode: {self.current_display_mode}")
+                logger.debug("Display active, processing mode: %s", self.current_display_mode)
                 
                 # Plugins update on their own schedules - no forced sync updates needed
                 # Each plugin has its own update_interval and background services
@@ -1803,7 +1807,7 @@ class DisplayController:
                         if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
                             should_skip = self.plugin_manager.health_tracker.should_skip_plugin(plugin_id)
                             if should_skip:
-                                logger.info(f"Skipping plugin {plugin_id} due to circuit breaker (mode: {active_mode})")
+                                logger.info("Skipping plugin %s due to circuit breaker (mode: %s)", plugin_id, active_mode)
                                 display_result = False
                                 # Skip to next mode - let existing logic handle it
                                 manager_to_display = None
@@ -1861,7 +1865,7 @@ class DisplayController:
                             if isinstance(result, bool):
                                 display_result = result
                                 if not display_result:
-                                    logger.info(f"Plugin {plugin_id} display() returned False for mode {active_mode}")
+                                    logger.info("Plugin %s display() returned False for mode %s", plugin_id, active_mode)
                         
                         # Record success if display completed without exception
                         if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
@@ -2354,6 +2358,16 @@ class DisplayController:
             Returns None on any error or if message is expired/invalid.
         """
         try:
+            # Throttle the existence stat to ~1 Hz: this runs on every render
+            # iteration (60+ fps), and the file usually doesn't exist — the
+            # status message's lifetime is measured in seconds anyway.
+            # Both attributes are initialised in __init__.
+            now = time.time()
+            if (now - self._wifi_status_check_ts) < 1.0:
+                return self._wifi_status_last_result
+            self._wifi_status_check_ts = now
+            self._wifi_status_last_result = None
+
             # Check if file exists
             if not self.wifi_status_file or not self.wifi_status_file.exists():
                 return None
@@ -2404,13 +2418,14 @@ class DisplayController:
                     pass
                 return None
             
-            # Message is valid and not expired
-            return {
+            # Message is valid and not expired — cache for the throttle window
+            self._wifi_status_last_result = {
                 'message': message,
                 'timestamp': timestamp,
                 'duration': duration,
                 'expires_at': expires_at
             }
+            return self._wifi_status_last_result
             
         except Exception as e:
             # Catch-all for any unexpected errors - log but don't break the display

src/plugin_system/plugin_loader.py

@@ -5,10 +5,10 @@ Handles plugin module imports, dependency installation, and class instantiation.
 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,101 @@ from pathlib import Path
 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
+
+
+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:
@@ -132,14 +221,14 @@ class PluginLoader:
                 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,
         plugin_id: str,
-        plugins_dir: Optional[Path] = None,
+        plugins_dir: Path,
         timeout: int = 300
     ) -> bool:
         """
@@ -148,7 +237,12 @@ class PluginLoader:
         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:
@@ -160,59 +254,42 @@ class PluginLoader:
 
         # Resolve to a canonical absolute path (normalises .. and symlinks)
         plugin_dir_real = os.path.realpath(str(plugin_dir))
+        plugins_dir_real = os.path.realpath(str(plugins_dir))
+        requested_name = os.path.basename(plugin_dir_real)
 
-        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
+        # 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")
-        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 +302,6 @@ class PluginLoader:
             )
 
             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 +313,7 @@ class PluginLoader:
                 # 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 +350,6 @@ class PluginLoader:
                             "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",
@@ -653,6 +717,14 @@ class PluginLoader:
         """
         # 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}",

src/plugin_system/plugin_manager.py

@@ -9,9 +9,9 @@ API Version: 1.0.0
 
 import json
 import sys
-import subprocess
 import time
 import threading
+import types
 from pathlib import Path
 from typing import Dict, List, Optional, Any
 import logging
@@ -177,90 +177,6 @@ class PluginManager:
 
         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.
@@ -828,8 +744,18 @@ class PluginManager:
                         # If resource monitor exists, wrap the call
                         def monitored_update():
                             self.resource_monitor.monitor_call(plugin_id, plugin_instance.update)
+                        # SimpleNamespace stores `update` as an *instance*
+                        # attribute, so attribute lookup returns the plain
+                        # function object as-is. A dynamically-built class
+                        # (`type(..., {'update': monitored_update})`) instead
+                        # stores it as a *class* attribute, which the
+                        # descriptor protocol turns into a bound method on
+                        # access -- silently prepending the instance as an
+                        # implicit first argument to a function that takes
+                        # none, raising "monitored_update() takes 0
+                        # positional arguments but 1 was given" on every call.
                         success = self.plugin_executor.execute_update(
-                            type('obj', (object,), {'update': monitored_update})(),
+                            types.SimpleNamespace(update=monitored_update),
                             plugin_id
                         )
                     else:

src/plugin_system/store_manager.py

@@ -5,7 +5,6 @@ Handles plugin discovery, installation, updates, and uninstallation
 from both the official registry and custom GitHub repositories.
 """
 
-import hashlib
 import os
 import re
 import json
@@ -26,6 +25,9 @@ import logging
 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, find_trusted_subdir
+)
 
 try:
     from jsonschema import Draft7Validator, ValidationError
@@ -1900,19 +1902,45 @@ class PluginStoreManager:
     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 +
+        # 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))
+        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"
+
         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 +1957,6 @@ class PluginStoreManager:
                 )
                 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 +2454,6 @@ class PluginStoreManager:
                                     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 +2464,9 @@ class PluginStoreManager:
                         )
                         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

src/plugin_system/testing/visual_display_manager.py

@@ -454,6 +454,18 @@ class VisualTestDisplayManager:
         """Check if display is currently scrolling."""
         return self._scrolling_state['is_scrolling']
 
+    def process_deferred_updates(self):
+        """Process any deferred updates (no-op for testing).
+
+        Several ticker-style plugins (news, odds-ticker, leaderboard,
+        stock-news, stocks) call this unconditionally between
+        set_scrolling_state() and their scroll-position update, mirroring the
+        real display_manager's deferred-update queue. This double has no such
+        queue, so there is nothing to process — the no-op just lets those
+        plugins render under the harness instead of raising AttributeError.
+        """
+        pass
+
     # ------------------------------------------------------------------
     # Utility methods
     # ------------------------------------------------------------------

src/vegas_mode/render_pipeline.py

@@ -66,6 +66,10 @@ class RenderPipeline:
             else display_manager.height
         )
 
+        # Reusable blank frame for cycle-end pushes (allocated lazily,
+        # re-blacked before each reuse)
+        self._blank_frame = None
+
         # ScrollHelper for optimized scrolling
         self.scroll_helper = ScrollHelper(
             self.display_width,
@@ -234,11 +238,19 @@ class RenderPipeline:
                     )
                     # Push blank immediately so the hardware never shows any
                     # post-wrap content while the coordinator recomposes the
-                    # next cycle (~100 ms).
+                    # next cycle (~100 ms). The blank is allocated once and
+                    # reused across cycle wraps (fresh paste each time in case
+                    # a consumer drew on the previous one).
                     try:
-                        from PIL import Image as _Image
-                        blank = _Image.new('RGB', (self.display_width, self.display_height))
-                        self.display_manager.image = blank
+                        if self._blank_frame is None or self._blank_frame.size != (
+                                self.display_width, self.display_height):
+                            self._blank_frame = Image.new(
+                                'RGB', (self.display_width, self.display_height))
+                        else:
+                            self._blank_frame.paste(
+                                (0, 0, 0),
+                                (0, 0, self.display_width, self.display_height))
+                        self.display_manager.image = self._blank_frame
                         self.display_manager.update_display()
                     except Exception:
                         logger.exception("Failed to write blank frame to display at cycle end")

test/plugins/test_visual_rendering.py

@@ -172,6 +172,16 @@ class TestVisualDisplayManager:
         vdm.set_scrolling_state(False)
         assert vdm.is_currently_scrolling() is False
 
+    def test_process_deferred_updates_is_noop(self):
+        # Ticker-style plugins (news, odds-ticker, leaderboard, stock-news,
+        # stocks) call this unconditionally alongside set_scrolling_state();
+        # it must exist and be harmless so those plugins render under the
+        # harness instead of raising AttributeError.
+        vdm = VisualTestDisplayManager(width=128, height=32)
+        vdm.set_scrolling_state(True)
+        vdm.process_deferred_updates()  # should not raise
+        assert vdm.is_currently_scrolling() is True
+
     def test_format_date_with_ordinal(self):
         from datetime import datetime
         vdm = VisualTestDisplayManager(width=128, height=32)

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

test/test_plugin_loader.py

@@ -193,7 +193,7 @@ class TestPluginLoader:
         
         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 @@ class TestPluginLoader:
         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()
@@ -216,20 +216,23 @@ class TestPluginLoader:
         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")
+        result = plugin_loader.install_dependencies(plugin_dir, "test_plugin", plugins_dir=tmp_plugins_dir)
 
         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"
@@ -242,16 +245,17 @@ class TestPluginLoader:
         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
         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."""
@@ -267,14 +271,15 @@ class TestPluginLoader:
         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
 
+    @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
@@ -293,7 +298,47 @@ class TestPluginLoader:
             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
+
+    @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", 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()

test/test_plugin_system.py

@@ -3,6 +3,7 @@ from unittest.mock import MagicMock, patch
 from pathlib import Path
 from src.plugin_system.plugin_manager import PluginManager
 from src.plugin_system.plugin_state import PluginState
+from src.plugin_system.resource_monitor import PluginResourceMonitor
 
 class TestPluginManager:
     """Test PluginManager functionality."""
@@ -74,18 +75,67 @@ class TestPluginManager:
             
             # No manifest in pm.plugin_manifests
             result = pm.load_plugin("non_existent_plugin")
-            
+
             assert result is False
             assert pm.state_manager.get_state("non_existent_plugin") == PluginState.ERROR
 
+    def test_run_scheduled_updates_calls_update_with_resource_monitor(
+        self, mock_config_manager, mock_display_manager, mock_cache_manager
+    ):
+        """Regression test: run_scheduled_updates() must actually call a
+        plugin's update() when self.resource_monitor is set (as it is in
+        every real deployment -- display_controller.py and web_interface/
+        app.py both assign a real PluginResourceMonitor after construction).
+
+        Previously, the resource_monitor branch wrapped the call in a
+        function stored as a *class* attribute on a dynamically-built type
+        (`type('obj', (object,), {'update': monitored_update})()`), which
+        the descriptor protocol turns into a bound method on access --
+        silently passing the synthetic instance as an implicit first
+        argument to monitored_update(), which takes none. Every plugin's
+        scheduled update failed with "monitored_update() takes 0 positional
+        arguments but 1 was given" and was silently swallowed into a
+        circuit-breaker retry loop that never succeeded, so plugin data
+        (scores, odds, etc.) never refreshed.
+        """
+        with patch('src.plugin_system.plugin_manager.ensure_directory_permissions'):
+            pm = PluginManager(
+                plugins_dir="plugins",
+                config_manager=mock_config_manager,
+                display_manager=mock_display_manager,
+                cache_manager=mock_cache_manager
+            )
+
+            plugin_instance = MagicMock()
+            plugin_instance.enabled = True
+            plugin_instance.update = MagicMock()
+
+            pm.plugins["test_plugin"] = plugin_instance
+            pm.plugin_manifests["test_plugin"] = {"update_interval": 10}
+            pm.state_manager.set_state("test_plugin", PluginState.ENABLED)
+            # Plain MagicMock, not the mock_cache_manager fixture: this test
+            # is about run_scheduled_updates() actually invoking update()
+            # through the resource-monitor wrapper, not about
+            # PluginResourceMonitor's own cache-backed metrics persistence
+            # (which calls cache_manager.get(..., memory_ttl=...) --
+            # a kwarg the fixture's mock_get() doesn't accept).
+            pm.resource_monitor = PluginResourceMonitor(MagicMock())
+
+            pm.run_scheduled_updates(current_time=time.time())
+
+            plugin_instance.update.assert_called_once()
+            assert "test_plugin" in pm.plugin_last_update
+            assert pm.state_manager.get_state("test_plugin") == PluginState.ENABLED
+
 
 class TestPluginLoader:
     """Test PluginLoader functionality."""
     
     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: