From 8de706323a46b7ed5286a1af8d7f4e32c1feae61 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sat, 4 Jul 2026 09:46:47 -0400 Subject: [PATCH 1/3] fix: install plugin/base requirements as root so ledmatrix.service can see them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ledmatrix-web.service runs as a non-root user, so "Reinstall plugin requirements" installed packages into that user's ~/.local site-packages. ledmatrix.service (the actual display, which loads and runs plugin code) runs as root and can't see another user's user-site packages, so plugins with dependencies not already present system-wide would silently fail at runtime with ModuleNotFoundError even after a "successful" reinstall. Reproduced and fixed live against a real device (weather plugin's astral dependency, used for moon-phase data): confirmed the exact failure ("No module named 'astral'" on every almanac cycle) and confirmed it's gone after this fix. Adds scripts/fix_perms/safe_pip_install.sh, a root-owned wrapper (mirroring the existing safe_plugin_rm.sh pattern) that validates the target is requirements.txt at the project root or under plugin-repos/ or plugins/ before running pip install as root. configure_web_sudo.sh provisions a narrowly-scoped sudoers rule for it. api_v3.py's install_base_requirements and install_plugin_requirements actions now use it via `sudo -n`, falling back to today's current-user-only install (with an explanatory note) if the wrapper isn't set up yet, so existing installs don't regress. Also uses --ignore-installed in the wrapper: root's site-packages often has apt/dpkg-managed copies of common libraries (requests, etc.) with no pip RECORD file, which pip refuses to upgrade in place and aborts the *entire* requirements.txt install over — discovered this while testing the fix live, since a plugin's other already-satisfied-for-the-web-user dependencies had never actually been attempted as root before. Also fixes a pre-existing bug in configure_web_sudo.sh where the display_controller.py/start_display.sh/stop_display.sh sudoers entries used PROJECT_DIR (scripts/install/, where this script lives) instead of PROJECT_ROOT (where those files actually live) — visible as the script's own "File access test" self-check failing. Verified fixed live. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --- scripts/fix_perms/safe_pip_install.sh | 74 +++++++++++++++++++++++++++ scripts/install/configure_web_sudo.sh | 33 ++++++++++-- web_interface/blueprints/api_v3.py | 46 ++++++++++++++--- 3 files changed, 140 insertions(+), 13 deletions(-) create mode 100755 scripts/fix_perms/safe_pip_install.sh diff --git a/scripts/fix_perms/safe_pip_install.sh b/scripts/fix_perms/safe_pip_install.sh new file mode 100755 index 000000000..ae7d266c8 --- /dev/null +++ b/scripts/fix_perms/safe_pip_install.sh @@ -0,0 +1,74 @@ +#!/bin/bash +# safe_pip_install.sh — Install a requirements.txt as root after validating +# that the resolved path is the project's own requirements.txt or a plugin's +# requirements.txt under plugin-repos/ or plugins/. +# +# This script is intended to be called via sudo from the web interface, so +# that packages a plugin declares end up visible to ledmatrix.service (which +# runs as root) rather than only to whichever non-root user runs the web +# interface. Plugin code already runs as root once loaded, so installing its +# declared dependencies as root is not a new trust boundary. +# +# Usage: safe_pip_install.sh + +set -euo pipefail + +if [ $# -ne 1 ]; then + echo "Usage: $0 " >&2 + exit 1 +fi + +TARGET="$1" + +# Determine the project root (parent of scripts/fix_perms/) +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" + +# Allowed locations (resolved, no trailing slash): +# - the project's own requirements.txt +# - any requirements.txt under plugin-repos/ or plugins/ +ALLOWED_EXACT="$(realpath --canonicalize-missing "$PROJECT_ROOT/requirements.txt")" +ALLOWED_BASES=( + "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugin-repos")" + "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugins")" +) + +# Resolve the target path (follow symlinks); works even if it doesn't exist. +RESOLVED_TARGET="$(realpath --canonicalize-missing "$TARGET")" + +# Must be named requirements.txt — never install from an arbitrary file. +if [ "$(basename "$RESOLVED_TARGET")" != "requirements.txt" ]; then + echo "DENIED: $RESOLVED_TARGET is not a requirements.txt file" >&2 + exit 2 +fi + +ALLOWED=false +if [ "$RESOLVED_TARGET" = "$ALLOWED_EXACT" ]; then + ALLOWED=true +else + for BASE in "${ALLOWED_BASES[@]}"; do + if [[ "$RESOLVED_TARGET" == "$BASE/"* ]]; then + ALLOWED=true + break + fi + done +fi + +if [ "$ALLOWED" = false ]; then + echo "DENIED: $RESOLVED_TARGET is not an allowed requirements.txt location" >&2 + echo "Allowed: $ALLOWED_EXACT, or any requirements.txt under: ${ALLOWED_BASES[*]}" >&2 + exit 2 +fi + +if [ ! -f "$RESOLVED_TARGET" ]; then + echo "ERROR: $RESOLVED_TARGET does not exist" >&2 + exit 3 +fi + +PYTHON_PATH="$(command -v python3)" +# --ignore-installed: root's site-packages often has apt/dpkg-managed copies +# of common libraries (requests, urllib3, ...) with no pip RECORD file, which +# pip refuses to uninstall in place ("Cannot uninstall: no RECORD file was +# found"). This tells pip to install the newer version alongside rather than +# aborting the whole requirements.txt install over one such conflict. +exec "$PYTHON_PATH" -m pip install --break-system-packages --ignore-installed -r "$RESOLVED_TARGET" diff --git a/scripts/install/configure_web_sudo.sh b/scripts/install/configure_web_sudo.sh index 0a9df2dab..feccc3e7b 100644 --- a/scripts/install/configure_web_sudo.sh +++ b/scripts/install/configure_web_sudo.sh @@ -33,6 +33,7 @@ POWEROFF_PATH=$(command -v poweroff) || true BASH_PATH=$(command -v bash) || true JOURNALCTL_PATH=$(command -v journalctl) || true SAFE_RM_PATH="$PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh" +SAFE_PIP_INSTALL_PATH="$PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh" # Validate required commands (systemctl, bash, python3 are essential) for CMD_NAME in SYSTEMCTL_PATH BASH_PATH PYTHON_PATH; do @@ -48,11 +49,15 @@ if [ ${#MISSING_CMDS[@]} -gt 0 ]; then exit 1 fi -# Validate helper script exists +# Validate helper scripts exist if [ ! -f "$SAFE_RM_PATH" ]; then echo "Error: Safe plugin removal helper not found: $SAFE_RM_PATH" >&2 exit 1 fi +if [ ! -f "$SAFE_PIP_INSTALL_PATH" ]; then + echo "Error: Safe pip install helper not found: $SAFE_PIP_INSTALL_PATH" >&2 + exit 1 +fi echo "Command paths:" echo " Python: $PYTHON_PATH" @@ -62,6 +67,7 @@ echo " Poweroff: ${POWEROFF_PATH:-(not found, skipping)}" echo " Bash: $BASH_PATH" echo " Journalctl: ${JOURNALCTL_PATH:-(not found, skipping)}" echo " Safe plugin rm: $SAFE_RM_PATH" +echo " Safe pip install: $SAFE_PIP_INSTALL_PATH" # Create a temporary sudoers file TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$" @@ -101,13 +107,22 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$" fi # Required: python3, bash - echo "$WEB_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_DIR/display_controller.py" - echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_DIR/start_display.sh" - echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_DIR/stop_display.sh" + # NOTE: display_controller.py/start_display.sh/stop_display.sh live at the + # project root, not under scripts/install/ (where this script lives) — + # must use PROJECT_ROOT here, not PROJECT_DIR. + echo "$WEB_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT/display_controller.py" + echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/start_display.sh" + echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/stop_display.sh" echo "" echo "# Allow web user to remove plugin directories via vetted helper script" echo "# The helper validates that the target path resolves inside plugin-repos/ or plugins/" echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_RM_PATH *" + echo "" + echo "# Allow web user to install a plugin's requirements.txt as root via vetted" + echo "# helper script, so packages are visible to root-run ledmatrix.service" + echo "# (not just the web interface's own user). The helper validates the target" + echo "# is requirements.txt at the project root or under plugin-repos/ or plugins/." + echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *" } > "$TEMP_SUDOERS" echo "" @@ -126,6 +141,7 @@ echo "- Run display_controller.py directly" echo "- Execute start_display.sh and stop_display.sh" echo "- Reboot and shutdown the system" echo "- Remove plugin directories (for update/uninstall when root-owned files block deletion)" +echo "- Install plugin/base requirements.txt as root (so ledmatrix.service can see them)" echo "" # Ask for confirmation @@ -147,6 +163,13 @@ fi if ! sudo chmod 755 "$SAFE_RM_PATH"; then echo "Warning: Could not set permissions on $SAFE_RM_PATH" fi +echo "Hardening safe_pip_install.sh ownership..." +if ! sudo chown root:root "$SAFE_PIP_INSTALL_PATH"; then + echo "Warning: Could not set ownership on $SAFE_PIP_INSTALL_PATH" +fi +if ! sudo chmod 755 "$SAFE_PIP_INSTALL_PATH"; then + echo "Warning: Could not set permissions on $SAFE_PIP_INSTALL_PATH" +fi if sudo cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web; then echo "Configuration applied successfully!" @@ -160,7 +183,7 @@ if sudo cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web; then echo "✗ systemctl status ledmatrix.service - Failed" fi - if sudo -n test -f "$PROJECT_DIR/start_display.sh"; then + if sudo -n test -f "$PROJECT_ROOT/start_display.sh"; then echo "✓ File access test - OK" else echo "✗ File access test - Failed" diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index bfa86ddb6..1c356cdb5 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -43,6 +43,42 @@ def _truncate_output(stdout: str, stderr: str) -> str: return combined +def _pip_install_requirements(req_file: Path, timeout: int) -> subprocess.CompletedProcess: + """Install a requirements.txt file, preferring the vetted sudo wrapper so + the packages are visible to root-run ledmatrix.service — not just to + whichever non-root user runs this web process. Falls back to installing + for the current process only if the wrapper isn't set up yet (i.e. the + admin hasn't run scripts/install/configure_web_sudo.sh since upgrading), + so the button still does *something* useful rather than hard-failing. + """ + wrapper = PROJECT_ROOT / 'scripts' / 'fix_perms' / 'safe_pip_install.sh' + if wrapper.exists(): + result = subprocess.run( + ['sudo', '-n', str(wrapper), str(req_file)], + capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT) + ) + if result.returncode == 0: + return result + note = ( + f"[Root install unavailable ({result.stderr.strip() or 'sudo denied'}); " + "installed for the web service'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: + note = ( + "[safe_pip_install.sh not found; installed for the web service's " + "user only. Run scripts/install/configure_web_sudo.sh to enable " + "root installs visible to ledmatrix.service.]\n" + ) + result = subprocess.run( + [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req_file)], + capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT) + ) + result.stdout = note + (result.stdout or '') + return result + + def _scrub_git_remote_url(url: str) -> str: """Strip embedded username/password from an HTTPS remote URL before returning it to the UI.""" try: @@ -1671,10 +1707,7 @@ def execute_system_action(): req_file = PROJECT_ROOT / 'requirements.txt' if not req_file.exists(): return jsonify({'status': 'error', 'message': 'No requirements.txt found at project root'}) - result = subprocess.run( - [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req_file)], - capture_output=True, text=True, timeout=120, cwd=str(PROJECT_ROOT) - ) + result = _pip_install_requirements(req_file, timeout=120) return jsonify({ 'status': 'success' if result.returncode == 0 else 'error', 'message': 'Base requirements installed successfully' if result.returncode == 0 else 'pip install failed', @@ -1695,10 +1728,7 @@ def execute_system_action(): req = p / 'requirements.txt' if p.is_dir() and req.exists(): try: - r = subprocess.run( - [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)], - capture_output=True, text=True, timeout=60 - ) + r = _pip_install_requirements(req, timeout=60) results.append({ 'plugin': p.name, 'ok': r.returncode == 0, From 7558aaab23e0d04969df43bfc9391fffa9eaa7ad Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sun, 5 Jul 2026 20:59:55 -0400 Subject: [PATCH 2/3] fix: invoke safe_pip_install.sh via explicit bash, matching sudoers rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught this on review: the sudoers rule configure_web_sudo.sh provisions is scoped to "$BASH_PATH $SAFE_PIP_INSTALL_PATH *" (matching the existing safe_plugin_rm.sh precedent in src/common/permission_utils.py), but _pip_install_requirements() called `sudo -n ` directly, relying on the script's shebang instead of an explicit bash prefix. sudo matches the literal command line, so this never matched the allowlisted rule on an install with only the specific sudoers entries this script provisions — it silently fell back to the non-root install path every time, which is the exact bug this PR set out to fix. This wasn't caught by live testing on ledpi.local because that device also has a broader, non-standard "NOPASSWD: ALL" grant which masked the mismatch. Confirmed the fix is correct by reading sudo's documented command-matching semantics and mirroring the already-proven-working bash-prefix pattern from permission_utils.py's safe_plugin_rm.sh call exactly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --- web_interface/blueprints/api_v3.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 1c356cdb5..1a506c8a9 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -53,8 +53,18 @@ def _pip_install_requirements(req_file: Path, timeout: int) -> subprocess.Comple """ wrapper = PROJECT_ROOT / 'scripts' / 'fix_perms' / 'safe_pip_install.sh' if wrapper.exists(): + # Must invoke via an explicit `bash ` — matching both the + # sudoers rule configure_web_sudo.sh provisions ($BASH_PATH + # $SAFE_PIP_INSTALL_PATH *) and the existing safe_plugin_rm.sh call + # in src/common/permission_utils.py. Calling the script path directly + # (relying on its shebang) makes sudo check a different command line + # than what's actually allowlisted, so `sudo -n` denies it on any + # install that only has the specific rules this script provisions — + # it only appeared to work in prior testing because that device also + # had a broader, non-standard NOPASSWD: ALL grant. + bash_path = shutil.which('bash') or '/bin/bash' result = subprocess.run( - ['sudo', '-n', str(wrapper), str(req_file)], + ['sudo', '-n', bash_path, str(wrapper), str(req_file)], capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT) ) if result.returncode == 0: From e454930fb6f3a854322b406f8254d827fa207416 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sun, 5 Jul 2026 21:08:55 -0400 Subject: [PATCH 3/3] fix: harden safe_pip_install.sh invocation against bash-path drift + address CodeRabbit nitpicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit follow-up findings on the bash-prefix fix (7558aaab): 1. (Actionable) shutil.which('bash') at runtime could in principle resolve to a different absolute path than configure_web_sudo.sh's `command -v bash`, which is resolved once at setup time and baked into the static sudoers file as a literal string — sudo requires an exact match. Now tries /usr/bin/bash and /bin/bash (the standard Debian/Raspberry Pi OS locations, matching what the setup script virtually always produces) before falling back to this process's own PATH resolution, so a divergence in just one of them doesn't break the install. 2. (Nitpick) Any nonzero returncode was treated as "sudo denied", so a real pip failure (bad package, build error) would trigger a pointless duplicate non-root install attempt and a misleading error message. Now distinguishes "sudo -n rejected this exact command line" from "sudo ran it but the command itself failed" via sudo's own diagnostic text, and surfaces genuine failures immediately without retrying other bash candidates or falling back. 3. (Nitpick) Added structured logging for every fallback/failure path (wrapper missing, sudo denied, real install failure), previously only visible via the returned stdout note — needed for remote debugging on a headless Pi. Verified: function-level smoke test confirms a real failure (this sandbox's system python3 lacking pip) is now correctly classified as non-denial and returned immediately without retrying candidates or double-installing. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --- web_interface/blueprints/api_v3.py | 54 ++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 1a506c8a9..790c21047 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -62,20 +62,60 @@ def _pip_install_requirements(req_file: Path, timeout: int) -> subprocess.Comple # install that only has the specific rules this script provisions — # it only appeared to work in prior testing because that device also # had a broader, non-standard NOPASSWD: ALL grant. - bash_path = shutil.which('bash') or '/bin/bash' - result = subprocess.run( - ['sudo', '-n', bash_path, str(wrapper), str(req_file)], - capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT) + # + # $BASH_PATH is resolved once at setup time (configure_web_sudo.sh's + # `command -v bash`) and baked into the static sudoers file as a + # literal path; sudo requires an exact string match against that, so + # if this process's own PATH resolves bash somewhere else, the + # sudoers rule won't match here either. Try the standard Debian/ + # Raspberry Pi OS locations first, then this process's own + # resolution, so a divergence in just one of them doesn't break this. + 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: + result = subprocess.run( + ['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 + # Best-effort distinction between "sudo rejected this exact + # command line" (no matching NOPASSWD rule for this bash path — + # worth trying the next candidate) and "sudo ran it but the + # wrapper/pip itself failed" (a real error — stop and surface it + # rather than uselessly retrying other bash paths or doubling up + # with a redundant non-root install attempt). + 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( + "[Pip Install] Root install failed (rc=%s) for %s: %s", + result.returncode, req_file, result.stderr.strip()[:500], + ) + return result + + logger.warning( + "[Pip Install] Root 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', ) - if result.returncode == 0: - return result note = ( - f"[Root install unavailable ({result.stderr.strip() or 'sudo denied'}); " + f"[Root install unavailable ({(result.stderr.strip() if result else 'sudo denied') or 'sudo denied'}); " "installed for the web service'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( + "[Pip Install] 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 web service's " "user only. Run scripts/install/configure_web_sudo.sh to enable "