mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
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
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
8de706323a
commit
7558aaab23
@@ -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 <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 = 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:
|
||||
|
||||
Reference in New Issue
Block a user