rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-08-01 16:58:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: dependency installation gaps in Plugin Store and first-time install (#385)

Browse Source 
* fix: install plugin dependencies through root-visible installer in Plugin Store

install_plugin/update_plugin (store_manager.py) installed requirements.txt
with a bare `pip3` off PATH, bypassing the root-visible installer added in
#380 for the "Reinstall Plugin Deps" button. Two bugs stacked: (1) `pip3`
can resolve to a different Python install than the one that actually runs
ledmatrix.service, and (2) even when it resolves correctly, ledmatrix-web
runs as a non-root user so the package lands in that user's local
site-packages, invisible to root-run ledmatrix.service. Either way the
install reports success and writes the .dependencies_installed hash marker,
so plugin_loader's own (correct) install-on-load path skips reinstalling —
leaving the dependency permanently missing until a user finds and clicks
the separate "Reinstall Plugin Deps" tool. This is why users kept hitting
"No module named 'astral'" for the weather plugin even after installing it
from the Store.

Extracts the sudo-wrapper-then-fallback install logic from api_v3.py's
_pip_install_requirements into src/common/permission_utils.py as
install_requirements_file, and routes store_manager.py's dependency
installation through it so the automatic Store install/update path now
matches the manual "Reinstall Plugin Deps" path.

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

* chore: suppress Codacy false-positive on subprocess.run in install_requirements_file

Codacy's generic subprocess-security rule (Bandit B603 equivalent) flagged
the pip/sudo subprocess.run calls in install_requirements_file for lacking a
"static string argument" — the standard pattern-based flag for any
subprocess.run() call with a variable in its argv list. Both calls use
list-form argv (no shell=True, so no shell-injection surface), and the only
dynamic value is req_file, a Path built internally by callers rather than
raw external input; safe_pip_install.sh independently re-validates it before
installing anything as root. Suppresses with inline `# nosec B603` comments
matching this codebase's existing convention (see permission_utils.py's own
PROTECTED_SYSTEM_DIRECTORIES, display_manager.py, sync_manager.py, etc.).

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

* fix: first-time install script fails on apt-managed requests package

web_interface/requirements.txt and requirements.txt both pin
requests>=2.33.0,<3.0.0, but Raspberry Pi OS ships an apt-managed
python3-requests with no pip RECORD file. Upgrading it via plain
`pip install` aborts with "uninstall-no-record-file" because pip refuses to
uninstall a package it has no record of, in place — which is exactly the
"Some web interface dependencies failed to install" warning first-time
install hits.

scripts/install_dependencies_apt.py and scripts/fix_perms/safe_pip_install.sh
already work around this with --ignore-installed (lets pip lay the new
version down in /usr/local, shadowing the apt copy, instead of trying to
remove it first). first_time_install.sh's own direct pip invocations —
the per-package requirements.txt loop, the web_interface/requirements.txt
install, and the requirements_web_v2.txt fallback — didn't have it. Adds
--ignore-installed to all three so first-time install no longer fails on
this well-known apt/pip conflict.

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

* chore: add nosemgrep to subprocess.run calls Codacy still flagged

The prior # nosec B603 comments suppressed Bandit's check but Codacy's
semgrep-based rule ("subprocess function 'run' without a static string")
kept flagging the same two lines as a critical security issue even after
that fix landed. install_dependencies_apt.py's _run() already needed both
tags together (# nosec B603 B607 ... # nosemgrep) for the identical
subprocess.run pattern, so apply the same double suppression here.

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

* fix: add --ignore-installed to install_requirements_file fallback path

CodeRabbit review caught this (confirming a gap already flagged in
conversation): the non-sudo fallback pip install in install_requirements_file
was missing --ignore-installed, unlike the sudo-wrapper branch and
safe_pip_install.sh. Without it, the same apt/pip RECORD-file conflict this
PR fixes elsewhere (first_time_install.sh, install_dependencies_apt.py) could
still hit installs that fall back to this path (e.g. a plugin's
requirements.txt on a host where safe_pip_install.sh isn't set up yet).

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-07-08 09:59:12 -04:00
committed by GitHub
co-authored by Claude Sonnet 5
parent 7a9d01342a
commit 63a233f3ed
4 changed files with 138 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/common/permission_utils.py
+102
View File
@@ -10,6 +10,7 @@ import os
import logging
import shutil as _shutil
import subprocess
import sys
from pathlib import Path
from typing import Optional
@@ -287,3 +288,104 @@ def sudo_remove_directory(path: Path, allowed_bases: Optional[list] = None) -> b
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)
)
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:
logger.warning(
"Root pip install failed (rc=%s) for %s: %s",
result.returncode, req_file, result.stderr.strip()[:500],
)
return result
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",
)
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 for %s",
req_file,
)
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.stdout = note + (result.stdout or "")
return result