Files
LEDMatrix/src/common/permission_utils.py
T
05e7c43b27 fix(plugins): replace dependency marker files with a real satisfaction check (#390)
* fix(plugins): replace dependency marker files with a real satisfaction check

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

* 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.

* 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).

* 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.

* 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).

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-11 08:55:50 -04:00

427 lines
16 KiB
Python

"""
Permission Utilities
Centralized utility functions for managing file and directory permissions
across the LEDMatrix codebase. Ensures consistent permission handling for
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
from pathlib import Path
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
'/tmp',
'/var/tmp',
'/dev',
'/proc',
'/sys',
'/run',
'/var/run',
'/etc',
'/boot',
'/var',
'/usr',
'/lib',
'/lib64',
'/bin',
'/sbin',
}
def ensure_directory_permissions(path: Path, mode: int = 0o775) -> None:
"""
Create directory and set permissions.
If the directory already exists and we cannot change its permissions,
we check if it's usable (readable/writable). If so, we continue without
raising an exception. This allows the system to work even when running
as a non-root user who cannot change permissions on existing directories.
Protected system directories (like /tmp, /etc, /var) are never modified
to prevent breaking system functionality.
Args:
path: Directory path to create/ensure
mode: Permission mode (default: 0o775 for group-writable directories)
Raises:
OSError: If directory creation fails or directory exists but is not usable
"""
try:
# Never modify permissions on system directories
path_str = str(path.resolve() if path.is_absolute() else path)
if path_str in PROTECTED_SYSTEM_DIRECTORIES:
logger.debug(f"Skipping permission modification on protected system directory: {path_str}")
# Verify the directory is usable
if path.exists() and os.access(path, os.R_OK | os.W_OK):
return
elif path.exists():
logger.warning(f"Protected system directory {path_str} exists but is not writable")
return
else:
raise OSError(f"Protected system directory {path_str} does not exist")
# Create directory if it doesn't exist
path.mkdir(parents=True, exist_ok=True)
# Try to set permissions
try:
os.chmod(path, mode)
logger.debug(f"Set directory permissions {oct(mode)} on {path}")
except (OSError, PermissionError) as perm_error:
# If we can't set permissions, check if directory is usable
if path.exists():
# Check if directory is readable and writable
if os.access(path, os.R_OK | os.W_OK):
logger.warning(
f"Could not set permissions on {path} (may be owned by different user), "
f"but directory is usable (readable/writable). Continuing."
)
return
else:
# Directory exists but is not usable
logger.error(
f"Directory {path} exists but is not readable/writable. "
f"Permission change failed: {perm_error}"
)
raise OSError(
f"Directory {path} exists but is not usable: {perm_error}"
) from perm_error
else:
# Directory doesn't exist and we couldn't create it
raise
except OSError as e:
logger.error(f"Failed to ensure directory {path}: {e}")
raise
def ensure_file_permissions(path: Path, mode: int = 0o644) -> None:
"""
Set file permissions after creation.
Args:
path: File path to set permissions on
mode: Permission mode (default: 0o644 for readable files)
Raises:
OSError: If permission setting fails
"""
try:
if path.exists():
os.chmod(path, mode)
logger.debug(f"Set file permissions {oct(mode)} on {path}")
else:
logger.warning(f"File does not exist, cannot set permissions: {path}")
except OSError as e:
logger.error(f"Failed to set file permissions on {path}: {e}")
raise
def get_config_file_mode(file_path: Path) -> int:
"""
Return appropriate permission mode for config files.
Args:
file_path: Path to config file
Returns:
Permission mode: 0o640 for secrets files, 0o644 for regular config
"""
if 'secrets' in str(file_path):
return 0o640 # rw-r-----
else:
return 0o644 # rw-r--r--
def get_assets_file_mode() -> int:
"""
Return permission mode for asset files (logos, images, etc.).
Returns:
Permission mode: 0o664 (rw-rw-r--) for group-writable assets
"""
return 0o664 # rw-rw-r--
def get_assets_dir_mode() -> int:
"""
Return permission mode for asset directories.
Returns:
Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
"""
return 0o2775 # rwxrwsr-x (setgid + group writable)
def get_config_dir_mode() -> int:
"""
Return permission mode for config directory.
Returns:
Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
"""
return 0o2775 # rwxrwsr-x (setgid + group writable)
def get_plugin_file_mode() -> int:
"""
Return permission mode for plugin files.
Returns:
Permission mode: 0o664 (rw-rw-r--) for group-writable plugin files
"""
return 0o664 # rw-rw-r--
def get_plugin_dir_mode() -> int:
"""
Return permission mode for plugin directories.
Returns:
Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
"""
return 0o2775 # rwxrwsr-x (setgid + group writable)
def get_cache_dir_mode() -> int:
"""
Return permission mode for cache directories.
Returns:
Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable cache directories
"""
return 0o2775 # rwxrwsr-x (setgid + group writable)
def sudo_remove_directory(path: Path, allowed_bases: Optional[list] = None) -> bool:
"""
Remove a directory using sudo as a last resort.
Used when normal removal fails due to root-owned files (e.g., __pycache__
directories created by the root ledmatrix service). Delegates to the
safe_plugin_rm.sh helper which validates the path is inside allowed
plugin directories.
Before invoking sudo, this function also validates that the resolved
path is a descendant of at least one allowed base directory.
Args:
path: Directory path to remove
allowed_bases: List of allowed parent directories. If None, defaults
to plugin-repos/ and plugins/ under the project root.
Returns:
True if removal succeeded, False otherwise
"""
# Determine project root (permission_utils.py is at src/common/)
project_root = Path(__file__).resolve().parent.parent.parent
if allowed_bases is None:
allowed_bases = [
project_root / "plugin-repos",
project_root / "plugins",
]
# Resolve the target path to prevent symlink/traversal tricks
try:
resolved = path.resolve()
except (OSError, ValueError) as e:
logger.error(f"Cannot resolve path {path}: {e}")
return False
# Validate the resolved path is a strict child of an allowed base
is_allowed = False
for base in allowed_bases:
try:
base_resolved = base.resolve()
if resolved != base_resolved and resolved.is_relative_to(base_resolved):
is_allowed = True
break
except (OSError, ValueError):
continue
if not is_allowed:
logger.error(
f"sudo_remove_directory DENIED: {resolved} is not inside "
f"allowed bases {[str(b) for b in allowed_bases]}"
)
return False
# Use the safe_plugin_rm.sh helper which does its own validation
helper_script = project_root / "scripts" / "fix_perms" / "safe_plugin_rm.sh"
if not helper_script.exists():
logger.error(f"Safe removal helper not found: {helper_script}")
return False
bash_path = _shutil.which('bash') or '/bin/bash'
try:
result = subprocess.run(
['sudo', '-n', bash_path, str(helper_script), str(resolved)],
capture_output=True,
text=True,
timeout=30
)
if result.returncode == 0 and not resolved.exists():
logger.info(f"Successfully removed {path} via sudo helper")
return True
else:
stderr = result.stderr.strip()
logger.error(f"sudo helper failed for {path}: {stderr}")
return False
except subprocess.TimeoutExpired:
logger.error(f"sudo helper timed out for {path}")
return False
except FileNotFoundError:
logger.error("sudo command not found on system")
return False
except Exception as e:
logger.error(f"Unexpected error during sudo helper for {path}: {e}")
return False
def install_requirements_file(req_file: Path, timeout: int = 300) -> subprocess.CompletedProcess:
"""
Install a requirements.txt file for a plugin (or the project itself).
Prefers the vetted sudo wrapper (scripts/fix_perms/safe_pip_install.sh) so
packages end up visible to root-run ledmatrix.service, not just to
whichever non-root user happens to run the calling process (e.g. the web
interface). Falls back to installing with the calling process's own
interpreter if the wrapper isn't set up yet (the admin hasn't run
scripts/install/configure_web_sudo.sh), so dependency installation still
does *something* useful rather than hard-failing.
Always installs with the interpreter that will actually run the code
(``sys.executable`` in the fallback path, the wrapper's ``python3`` in the
sudo path) rather than a bare ``pip``/``pip3`` off PATH, which can
silently resolve to a different Python installation (e.g. system Python
vs. a virtualenv) than the one importing the package at runtime.
Args:
req_file: Path to a requirements.txt file
timeout: Subprocess timeout in seconds
Returns:
subprocess.CompletedProcess from the pip (or wrapper) invocation.
Never raises on a non-zero exit; callers should check ``returncode``.
``stdout`` is prefixed with an explanatory note when the root wrapper
was unavailable and the fallback path was used.
"""
project_root = Path(__file__).resolve().parent.parent.parent
wrapper = project_root / "scripts" / "fix_perms" / "safe_pip_install.sh"
if wrapper.exists():
# See sudo_remove_directory / configure_web_sudo.sh for why bash must
# be invoked with an explicit, known path rather than relying on the
# wrapper's shebang: sudoers matches the exact command line.
bash_candidates = []
for candidate in ("/usr/bin/bash", "/bin/bash", _shutil.which("bash")):
if candidate and candidate not in bash_candidates:
bash_candidates.append(candidate)
result = None
for bash_path in bash_candidates:
# bash_path and wrapper are fixed, known-good paths, and
# safe_pip_install.sh independently re-validates req_file is an
# allowed requirements.txt before installing anything as root.
result = subprocess.run( # nosec B603 - no shell invoked (list-form argv) # nosemgrep
["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
# trying the next bash candidate) from "sudo ran it but pip
# itself failed" (a real error — stop and surface it).
denied = any(
phrase in result.stderr
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); 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 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'}); "
"installed for the current process's user only. Packages may not be "
"visible to ledmatrix.service if it runs as a different user — "
"run scripts/install/configure_web_sudo.sh to fix this.]\n"
)
else:
logger.warning(
"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 "
"user only. Run scripts/install/configure_web_sudo.sh to enable "
"root installs visible to ledmatrix.service.]\n"
)
# sys.executable is this process's own interpreter (not
# attacker-influenced), and req_file is a Path built internally by callers
# (store_manager.py plugin paths, PROJECT_ROOT/requirements.txt), never
# raw external/user input. --ignore-installed matches safe_pip_install.sh:
# apt-managed packages (e.g. python3-requests) ship no pip RECORD file, so
# upgrading them would otherwise abort with "uninstall-no-record-file".
result = subprocess.run( # nosec B603 - no shell invoked (list-form argv) # nosemgrep
[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.stderr = _redact_url_credentials(result.stderr)
result.stdout = note + _redact_url_credentials(result.stdout)
return result