diff --git a/CHANGELOG.md b/CHANGELOG.md index 51af35f..caa1a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -213,6 +213,12 @@ considered stable; breaking changes to them will come with a major version bump. ### Fixed +- Service installers no longer leave `venv/bin/pip` (and other console scripts) + with shebangs pointing at the temporary `.venv-build-$$` path after the atomic + virtualenv swap. Optional package prompts and documented pip invocations now + use `venv/bin/python -m pip`. `--update-venv` rewrites shebangs in place so + already-broken installs heal without a full rebuild, and the previous venv is + kept until rewrite succeeds (issue #229). - Data retention now runs shortly after startup and then daily. It no longer requires 24 hours of uninterrupted uptime before the first cleanup, and its timer remains independent from the nightly maintenance email. diff --git a/docs/service-installation.md b/docs/service-installation.md index 338d0ae..501759d 100644 --- a/docs/service-installation.md +++ b/docs/service-installation.md @@ -99,7 +99,7 @@ tree writable by the service account. ```bash sudo python3 -m venv /opt/meshcore-bot/venv -sudo /opt/meshcore-bot/venv/bin/pip install \ +sudo /opt/meshcore-bot/venv/bin/python -m pip install \ -r /opt/meshcore-bot/requirements.txt ``` @@ -216,7 +216,7 @@ sudo systemctl restart meshcore-bot 1. Check service status: `sudo systemctl status meshcore-bot` 2. View logs: `sudo journalctl -u meshcore-bot -n 50` 3. Check configuration: `sudo nano /etc/meshcore-bot/config.ini` -4. Verify dependencies: `/opt/meshcore-bot/venv/bin/pip list | grep meshcore` +4. Verify dependencies: `/opt/meshcore-bot/venv/bin/python -m pip list | grep meshcore` ### Dependency Import or Syntax Errors diff --git a/install-service.sh b/install-service.sh index 01840b3..9bd1c67 100755 --- a/install-service.sh +++ b/install-service.sh @@ -412,9 +412,12 @@ configure_armv7_pip_args() { # upgrades only what no longer satisfies a specifier, which is the fast path. # `--upgrade` would eagerly churn transitive dependencies on every run. Extras # installed previously (profanity filter, geocoding) survive, unlike a rebuild. +# Also rewrites console-script shebangs so an earlier .venv-build-$$ relocate +# does not leave venv/bin/pip broken after --update-venv (issue #229). update_venv_in_place() { local venv="$1" local requirements="$2" + local rewrite_helper print_info "Reusing the existing virtualenv at $venv" print_info "Synchronizing dependencies from $requirements" @@ -425,6 +428,19 @@ update_venv_in_place() { return 1 fi + # Prefer the helper shipped with this installer invocation so --update-venv + # heals broken shebangs even before a full code sync lands under INSTALL_DIR. + if [ -f "$SCRIPT_DIR/scripts/rewrite_venv_shebangs.sh" ]; then + rewrite_helper="$SCRIPT_DIR/scripts/rewrite_venv_shebangs.sh" + else + rewrite_helper="$INSTALL_DIR/scripts/rewrite_venv_shebangs.sh" + fi + if ! bash "$rewrite_helper" "$venv"; then + print_error "Failed to rewrite virtualenv console-script shebangs" + print_info "Rebuild with: $0 --upgrade" + return 1 + fi + harden_venv_permissions "$venv" print_success "Dependencies are up to date in the existing virtualenv" } @@ -780,12 +796,26 @@ if [[ "$VENV_UPDATED_IN_PLACE" != true ]]; then print_error "Failed to activate the newly built virtual environment" exit 1 fi + # Console scripts still embed the .venv-build-$$ shebang; rewrite before any + # later call to venv/bin/pip or other entry points (see issue #229). Keep + # VENV_OLD until rewrite succeeds so a failure can restore the prior tree. + if ! bash "$INSTALL_DIR/scripts/rewrite_venv_shebangs.sh" "$INSTALL_DIR/venv"; then + print_error "Failed to rewrite virtualenv console-script shebangs after relocate" + if [ -d "$VENV_OLD" ]; then + rm -rf "$INSTALL_DIR/venv" + mv "$VENV_OLD" "$INSTALL_DIR/venv" + print_warning "Restored the previous virtualenv after shebang rewrite failure" + fi + exit 1 + fi rm -rf "$VENV_OLD" print_success "Installed all Python dependencies into a fresh virtual environment" fi # Optional extras. A rebuild starts empty so these have to be re-chosen; an # in-place update keeps whatever was installed before, so don't re-prompt. +# Always invoke pip via `python -m pip` so a stale shebang cannot break installs. +VENV_PYTHON="$INSTALL_DIR/venv/bin/python" if [[ "$VENV_UPDATED_IN_PLACE" == true ]]; then print_info "Kept any optional packages already installed in the virtualenv" else @@ -797,20 +827,22 @@ else if ask_yes_no "Install profanity filter packages? (recommended if using the profanity filter feature)" "n"; then print_info "Installing profanity filter packages..." - "$INSTALL_DIR/venv/bin/pip" install --quiet "better-profanity>=0.7.0" "unidecode>=1.3.0" || { + if "$VENV_PYTHON" -m pip install --quiet "better-profanity>=0.7.0" "unidecode>=1.3.0"; then + print_success "Installed profanity filter packages" + else print_warning "Failed to install profanity filter packages (non-fatal)" - } - print_success "Installed profanity filter packages" + fi else print_info "Skipping profanity filter packages" fi if ask_yes_no "Install geocoding extras? (recommended if using location/path commands)" "n"; then print_info "Installing geocoding extras..." - "$INSTALL_DIR/venv/bin/pip" install --quiet "pycountry>=23.12.0" "us>=2.0.0" || { + if "$VENV_PYTHON" -m pip install --quiet "pycountry>=23.12.0" "us>=2.0.0"; then + print_success "Installed geocoding extras" + else print_warning "Failed to install geocoding extras (non-fatal)" - } - print_success "Installed geocoding extras" + fi else print_info "Skipping geocoding extras" fi diff --git a/scripts/build-deb.sh b/scripts/build-deb.sh index f019b6c..32d22f0 100755 --- a/scripts/build-deb.sh +++ b/scripts/build-deb.sh @@ -235,8 +235,8 @@ VENV_OLD="${INSTALL_ROOT}/.venv-old-$$" rm -rf "${VENV_BUILD}" "${VENV_OLD}" echo "Building fresh Python virtualenv…" python3 -m venv "${VENV_BUILD}" -"${VENV_BUILD}/bin/pip" install --quiet --upgrade pip -"${VENV_BUILD}/bin/pip" install --quiet -r "${INSTALL_ROOT}/requirements.txt" +"${VENV_BUILD}/bin/python" -m pip install --quiet --upgrade pip +"${VENV_BUILD}/bin/python" -m pip install --quiet -r "${INSTALL_ROOT}/requirements.txt" if [ -d "${INSTALL_ROOT}/venv" ]; then mv "${INSTALL_ROOT}/venv" "${VENV_OLD}" fi @@ -244,6 +244,15 @@ if ! mv "${VENV_BUILD}" "${INSTALL_ROOT}/venv"; then [ -d "${VENV_OLD}" ] && mv "${VENV_OLD}" "${INSTALL_ROOT}/venv" exit 1 fi +# Shebangs still point at .venv-build-$$; rewrite for the final path (#229). +# Keep VENV_OLD until rewrite succeeds so a failure can restore the prior tree. +if ! bash "${INSTALL_ROOT}/scripts/rewrite_venv_shebangs.sh" "${INSTALL_ROOT}/venv"; then + if [ -d "${VENV_OLD}" ]; then + rm -rf "${INSTALL_ROOT}/venv" + mv "${VENV_OLD}" "${INSTALL_ROOT}/venv" + fi + exit 1 +fi rm -rf "${VENV_OLD}" # A legacy prerm destroys the prior state before new package code can observe diff --git a/scripts/rewrite_venv_shebangs.sh b/scripts/rewrite_venv_shebangs.sh new file mode 100755 index 0000000..60067b8 --- /dev/null +++ b/scripts/rewrite_venv_shebangs.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Rewrite absolute shebangs in a virtualenv's bin/ scripts after the tree has +# been relocated. `python -m venv` and pip bake the creation path into console +# scripts; an atomic mv from .venv-build-$$ to venv/ leaves those shebangs +# pointing at a path that no longer exists ("cannot execute: required file not +# found"). Symlinked interpreters (python, python3, …) are left alone. +# +# Usage: rewrite_venv_shebangs.sh /path/to/venv +set -euo pipefail + +venv="${1:?usage: $0 /path/to/venv}" +new_python="$venv/bin/python" + +if [ ! -x "$new_python" ]; then + echo "rewrite_venv_shebangs: no usable interpreter at $new_python" >&2 + exit 1 +fi + +shopt -s nullglob +for script in "$venv/bin"/*; do + [ -f "$script" ] || continue + [ -L "$script" ] && continue + + first="$(head -n 1 "$script" 2>/dev/null || true)" + case "$first" in + "#!"*"python"*) + if [ "$first" = "#!$new_python" ]; then + continue + fi + { + printf '#!%s\n' "$new_python" + tail -n +2 "$script" + } >"$script.tmp" + mv "$script.tmp" "$script" + chmod a+rx "$script" + ;; + esac +done diff --git a/tests/test_service_packaging.py b/tests/test_service_packaging.py index 27a5605..edeeaec 100644 --- a/tests/test_service_packaging.py +++ b/tests/test_service_packaging.py @@ -7,6 +7,7 @@ import os import shutil import sqlite3 import subprocess +import sys from pathlib import Path from scripts.migrate_service_layout import migrate_service_layout @@ -45,6 +46,14 @@ def test_service_units_do_not_make_code_tree_writable(): assert 'find "${CONF_DIR}" "${DATA_DIR}" -type f -exec chmod 0600' in deb_builder assert 'VENV_BUILD="${INSTALL_ROOT}/.venv-build-$$"' in deb_builder assert 'if [ ! -d "${INSTALL_ROOT}/venv" ]' not in deb_builder + assert 'scripts/rewrite_venv_shebangs.sh" "${INSTALL_ROOT}/venv"' in deb_builder + assert '/bin/python" -m pip install --quiet --upgrade pip' in deb_builder + assert "/bin/pip\" install" not in deb_builder + assert ( + 'mv "${VENV_OLD}" "${INSTALL_ROOT}/venv"' in deb_builder + and deb_builder.index('rewrite_venv_shebangs.sh" "${INSTALL_ROOT}/venv"') + < deb_builder.index('rm -rf "${VENV_OLD}"') + ) assert 'scripts/debian_service_state.sh" "${BUILD_DIR}/DEBIAN/preinst"' in deb_builder assert 'debian_service_state.sh" restore "${2:-}"' in deb_builder assert 'debian_service_state.sh preremove "${1:-}"' in deb_builder @@ -62,6 +71,19 @@ def test_standalone_installer_separates_code_and_private_state(): assert "source-authoritative secure install" in installer assert "preserve_service_alternatives.py" in installer assert 'VENV_BUILD="$INSTALL_DIR/.venv-build-$$"' in installer + assert 'rewrite_venv_shebangs.sh" "$INSTALL_DIR/venv"' in installer + assert 'bash "$rewrite_helper" "$venv"' in installer + assert "update_venv_in_place" in installer + assert installer.index("update_venv_in_place()") < installer.index( + 'bash "$rewrite_helper" "$venv"' + ) + assert installer.index('bash "$rewrite_helper" "$venv"') < installer.index( + 'harden_venv_permissions "$venv"' + ) + assert "Restored the previous virtualenv after shebang rewrite failure" in installer + assert '"$VENV_PYTHON" -m pip install --quiet "better-profanity' in installer + assert '"$VENV_PYTHON" -m pip install --quiet "pycountry' in installer + assert '"$INSTALL_DIR/venv/bin/pip"' not in installer assert 'Preserving existing virtual environment' not in installer assert "Python 3.10+ installed" in installer assert "sys.version_info < (3, 10)" in installer @@ -79,6 +101,49 @@ def test_standalone_installer_separates_code_and_private_state(): assert "Refusing to restart a potentially partial code tree" in installer +def test_relocated_venv_shebangs_are_rewritten(tmp_path: Path) -> None: + """Regression for issue #229: atomic venv mv leaves broken console shebangs.""" + build = tmp_path / ".venv-build-test" + final = tmp_path / "venv" + subprocess.run([sys.executable, "-m", "venv", str(build)], check=True) + + pip_build = build / "bin" / "pip" + assert pip_build.is_file() + shebang_before = pip_build.read_text(encoding="utf-8").splitlines()[0] + assert str(build) in shebang_before + + build.rename(final) + pip_final = final / "bin" / "pip" + # After relocate, the baked shebang still points at the old build path. + assert str(build) in pip_final.read_text(encoding="utf-8").splitlines()[0] + try: + broken = subprocess.run( + [str(pip_final), "--version"], + capture_output=True, + text=True, + check=False, + ) + assert broken.returncode != 0 + except FileNotFoundError: + # macOS/execve reports a missing shebang interpreter as ENOENT on the + # script path itself rather than a non-zero exit from pip. + pass + + rewrite = REPO_ROOT / "scripts" / "rewrite_venv_shebangs.sh" + subprocess.run(["bash", str(rewrite), str(final)], check=True) + + shebang_after = pip_final.read_text(encoding="utf-8").splitlines()[0] + assert shebang_after == f"#!{final / 'bin' / 'python'}" + fixed = subprocess.run( + [str(pip_final), "--version"], + capture_output=True, + text=True, + check=False, + ) + assert fixed.returncode == 0, fixed.stderr + assert "pip" in fixed.stdout.lower() + + def test_service_documentation_matches_hardened_layout() -> None: readme = (REPO_ROOT / "README.md").read_text(encoding="utf-8") getting_started = (REPO_ROOT / "docs/getting-started.md").read_text(encoding="utf-8") @@ -96,6 +161,9 @@ def test_service_documentation_matches_hardened_layout() -> None: assert "sudo chown -R meshcore:meshcore /opt/meshcore-bot" not in service_docs assert "patches the meshcore file" not in service_docs assert "/var/lib/meshcore-bot" in service_docs + assert "venv/bin/python -m pip" in service_docs + assert "venv/bin/pip install" not in service_docs + assert "venv/bin/pip list" not in service_docs assert "Upgrading from v0.9.3 to v1.0.0" in upgrade_docs