Fix: plugin/base requirements installed as web-user, invisible to root-run display service (#380)

* fix: install plugin/base requirements as root so ledmatrix.service can see them

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

* fix: invoke safe_pip_install.sh via explicit bash, matching sudoers rule

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

* fix: harden safe_pip_install.sh invocation against bash-path drift + address CodeRabbit nitpicks

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-07-06 08:04:49 -04:00
committed by GitHub
co-authored by Claude Sonnet 5
parent c6ce332d49
commit cbb8ec41e8
3 changed files with 190 additions and 13 deletions
+88 -8
View File
@@ -43,6 +43,92 @@ 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():
# Must invoke via an explicit `bash <path>` — 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 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',
)
note = (
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 "
"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 +1757,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 +1778,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,