From e75b99f7871355d8df124fa72dbf8f9d73f43885 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:50:33 -0400 Subject: [PATCH] fix(update): let the root pip wrapper install web_interface/requirements.txt Update Code, the automatic update's health check and Install Base Requirements install web_interface/requirements.txt through safe_pip_install.sh, which only allowed the root requirements.txt. The first commit changing that file would fail its dependency install, and the automatic updater rolls back any update whose dependencies did not install -- on every device, for every newer commit. The wrapper now lists both core requirement files. Only their folders are resolved, so a requirements.txt symlinked out of the project is compared by its target and refused (previously the root file's own symlink target was what got allowed). The updater's file list is a named constant, and a test runs the real wrapper (pip stubbed) on every file Update Code and the rollback install. Co-Authored-By: Claude Opus 5 --- scripts/fix_perms/safe_pip_install.sh | 27 +++-- test/test_safe_pip_install_allowlist.py | 128 ++++++++++++++++++++++ web_interface/blueprints/api_v3/system.py | 8 +- 3 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 test/test_safe_pip_install_allowlist.py diff --git a/scripts/fix_perms/safe_pip_install.sh b/scripts/fix_perms/safe_pip_install.sh index ae7d266c..06557e6f 100755 --- a/scripts/fix_perms/safe_pip_install.sh +++ b/scripts/fix_perms/safe_pip_install.sh @@ -1,6 +1,7 @@ #!/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 +# that the resolved path is one of the project's own requirements files +# (requirements.txt, web_interface/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 @@ -25,9 +26,17 @@ 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 +# - the project's own requirements files. Update Code, the automatic +# update's health check and Install Base Requirements install both, so +# one missing here is refused on every device -- and the automatic +# updater rolls back any update that changes it. +# Only their folders are resolved: resolving the files too would follow +# a requirements.txt symlinked out of the project and allow its target. # - any requirements.txt under plugin-repos/ or plugins/ -ALLOWED_EXACT="$(realpath --canonicalize-missing "$PROJECT_ROOT/requirements.txt")" +ALLOWED_EXACT=( + "$(realpath --canonicalize-missing "$PROJECT_ROOT")/requirements.txt" + "$(realpath --canonicalize-missing "$PROJECT_ROOT/web_interface")/requirements.txt" +) ALLOWED_BASES=( "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugin-repos")" "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugins")" @@ -43,9 +52,13 @@ if [ "$(basename "$RESOLVED_TARGET")" != "requirements.txt" ]; then fi ALLOWED=false -if [ "$RESOLVED_TARGET" = "$ALLOWED_EXACT" ]; then - ALLOWED=true -else +for EXACT in "${ALLOWED_EXACT[@]}"; do + if [ "$RESOLVED_TARGET" = "$EXACT" ]; then + ALLOWED=true + break + fi +done +if [ "$ALLOWED" = false ]; then for BASE in "${ALLOWED_BASES[@]}"; do if [[ "$RESOLVED_TARGET" == "$BASE/"* ]]; then ALLOWED=true @@ -56,7 +69,7 @@ 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 + echo "Allowed: ${ALLOWED_EXACT[*]}, or any requirements.txt under: ${ALLOWED_BASES[*]}" >&2 exit 2 fi diff --git a/test/test_safe_pip_install_allowlist.py b/test/test_safe_pip_install_allowlist.py new file mode 100644 index 00000000..c907e14b --- /dev/null +++ b/test/test_safe_pip_install_allowlist.py @@ -0,0 +1,128 @@ +"""scripts/fix_perms/safe_pip_install.sh must accept what the updaters install. + +Update Code and the automatic update's health check install the core's own +requirement files through that root wrapper, and the wrapper refuses any path +it does not list. It used to list only requirements.txt, so an update that +changed web_interface/requirements.txt failed its dependency install -- and +the automatic updater rolls back any update whose dependencies did not +install, on every device, every week. + +The real script runs here with only its final pip command swapped for an +echo, so the path checks under test are the shipped ones. +""" +import importlib.util +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT)) + +WRAPPER_REL = Path('scripts') / 'fix_perms' / 'safe_pip_install.sh' +PIP_LINE = 'exec "$PYTHON_PATH" -m pip install' + + +def _bash(): + """A bash whose realpath understands --canonicalize-missing, or None.""" + candidates = [shutil.which('bash')] + if os.name == 'nt': + candidates.insert(0, r'C:\Program Files\Git\bin\bash.exe') + for bash in candidates: + if not bash or not os.path.exists(bash): + continue + try: + probe = subprocess.run([bash, '-c', 'realpath --canonicalize-missing /no/such/x'], + capture_output=True, text=True, timeout=30) + except (OSError, subprocess.SubprocessError): + continue + if probe.returncode == 0: + return bash + return None + + +BASH = _bash() +pytestmark = pytest.mark.skipif(BASH is None, reason='no bash with GNU realpath') + + +@pytest.fixture +def project(tmp_path): + """A project tree holding the real wrapper, with pip stubbed out.""" + root = tmp_path / 'LEDMatrix' + wrapper = root / WRAPPER_REL + wrapper.parent.mkdir(parents=True) + text = (ROOT / WRAPPER_REL).read_text(encoding='utf-8').replace('\r\n', '\n') + lines = text.split('\n') + pip = [i for i, line in enumerate(lines) if line.startswith(PIP_LINE)] + assert len(pip) == 1, 'the wrapper no longer ends in the pip install this test stubs' + lines[pip[0]] = 'echo "WOULD INSTALL $RESOLVED_TARGET"; exit 0' + wrapper.write_text('\n'.join(lines), encoding='utf-8', newline='\n') + for rel in ('requirements.txt', 'web_interface/requirements.txt', + 'plugin-repos/clock/requirements.txt', 'src/requirements.txt', + 'web_interface/extra/requirements.txt'): + (root / rel).parent.mkdir(parents=True, exist_ok=True) + (root / rel).write_text('requests\n', encoding='utf-8') + return root + + +def run_wrapper(root, rel): + """Run the wrapper on ``rel``, with the path spelled the way bash sees the tree.""" + return subprocess.run( + [BASH, '-c', 'cd "$1" && bash scripts/fix_perms/safe_pip_install.sh "$PWD/$2"', + '_', str(root), rel], + capture_output=True, text=True, timeout=60) + + +def _core_requirement_files(): + from web_interface.blueprints.api_v3 import system + spec = importlib.util.spec_from_file_location( + 'auto_update_verify_for_allowlist', ROOT / 'scripts' / 'utils' / 'auto_update_verify.py') + verifier = importlib.util.module_from_spec(spec) + spec.loader.exec_module(verifier) + assert set(verifier.REQUIREMENT_FILES) == set(system.CORE_REQUIREMENT_FILES), ( + 'the rollback must reinstall the same files Update Code installs') + return system.CORE_REQUIREMENT_FILES + + +@pytest.mark.parametrize('rel', _core_requirement_files()) +def test_every_core_requirement_file_the_updaters_install_is_allowed(project, rel): + result = run_wrapper(project, rel) + assert result.returncode == 0, result.stderr + assert 'WOULD INSTALL' in result.stdout + + +def test_plugin_requirements_are_still_allowed(project): + assert run_wrapper(project, 'plugin-repos/clock/requirements.txt').returncode == 0 + + +@pytest.mark.parametrize('rel', [ + 'src/requirements.txt', # repo-owned folder, but not listed + 'web_interface/extra/requirements.txt', # below an allowed file's folder + 'web_interface/requirements.txt.bak', # not a requirements.txt +]) +def test_other_paths_are_still_refused(project, rel): + if rel.endswith('.bak'): + (project / rel).write_text('requests\n', encoding='utf-8') + result = run_wrapper(project, rel) + assert result.returncode == 2, result.stdout + result.stderr + assert 'DENIED' in result.stderr + + +@pytest.mark.parametrize('rel', ['requirements.txt', 'web_interface/requirements.txt']) +def test_a_core_requirement_file_symlinked_out_of_the_project_is_refused(project, tmp_path, rel): + """Only the allowed files' folders are resolved, so the link's target is + compared, and refused, rather than allowed as the file itself.""" + outside = tmp_path / 'elsewhere' / 'requirements.txt' + outside.parent.mkdir() + outside.write_text('evil\n', encoding='utf-8') + link = project / rel + link.unlink() + try: + link.symlink_to(outside) + except (OSError, NotImplementedError): + pytest.skip('symlinks unavailable') + result = run_wrapper(project, rel) + assert result.returncode == 2, result.stdout + result.stderr diff --git a/web_interface/blueprints/api_v3/system.py b/web_interface/blueprints/api_v3/system.py index 2c5d712a..0ba98a1c 100644 --- a/web_interface/blueprints/api_v3/system.py +++ b/web_interface/blueprints/api_v3/system.py @@ -228,6 +228,12 @@ def _sudo_hint_for(text): _core_update_lock = threading.Lock() +#: The core's own requirement files, installed after a pull that changes them. +#: scripts/fix_perms/safe_pip_install.sh must accept every one (it refuses +#: anything it does not list), and scripts/utils/auto_update_verify.py +#: reinstalls the same files when it rolls an update back. +CORE_REQUIREMENT_FILES = ('requirements.txt', 'web_interface/requirements.txt') + def perform_core_update(): """Pull the latest LEDMatrix code and sync its dependencies. @@ -369,7 +375,7 @@ def _perform_core_update_locked(): ['git', 'diff', '--name-only', f'{old_head}..{new_head}'], capture_output=True, text=True, timeout=15, cwd=project_dir) changed = set(diff.stdout.split()) if diff.returncode == 0 else set() - for rel in ('requirements.txt', 'web_interface/requirements.txt'): + for rel in CORE_REQUIREMENT_FILES: req_path = PROJECT_ROOT / rel if rel not in changed or not req_path.exists(): continue