fix(updater): pre-main hardening (dead-branch drop, reclone rung, auto-provision, klipper ownership)

scripts/BlocksScreen-deploy.service

@@ -6,6 +6,8 @@ DefaultDependencies=no
 Type=oneshot
 User=root
 ExecStart=/bin/bash BS_DIR/scripts/install-updater.sh
-ExecStartPost=/bin/rm -f BS_PRIMARY_HOME/.config/blockscreen/.run-install-updater
+# ExecStopPost also runs when ExecStart fails: a persistent failure must not
+# leave the flag set and storm the path unit into its start limit.
+ExecStopPost=-/bin/rm -f BS_PRIMARY_HOME/.config/blockscreen/.run-install-updater
 StandardOutput=journal
 StandardError=journal

scripts/BlocksScreen-start.sh

@@ -92,6 +92,16 @@ git -C "$BS_PATH" config core.hooksPath scripts 2>/dev/null || true
 
 bs_migrate_moonraker_conf "$_BSENV_HOME/printer_data/config/moonraker.conf" BlocksScreen-start
 
+# Serialize the boot git surgery below with the updater daemon/CLI (their flock):
+# the daemon's boot reconcile can heal this same repo concurrently. Proceed after
+# 20s rather than hold up the UI; the fd is closed before exec'ing the UI.
+_BS_LOCK_DIR="/run/blockscreen"
+mkdir -p "$_BS_LOCK_DIR" 2>/dev/null || _BS_LOCK_DIR="$_BSENV_HOME/.cache/blockscreen"
+if exec 9>"$_BS_LOCK_DIR/updater.lock" 2>/dev/null; then
+    flock -w 20 9 2>/dev/null \
+        || echo "[BlocksScreen-start] updater lock busy - proceeding without it"
+fi
+
 # Remove stale git index lock left by an interrupted update (e.g. power loss during git reset)
 rm -f "$BS_PATH/.git/index.lock"
 
@@ -148,6 +158,9 @@ if ! "$BSENV/bin/python3.11" -m compileall -q \
     git -C "$BS_PATH" reset --hard HEAD 2>/dev/null || true
 fi
 
+# Release the updater lock now: the exec'd UI must never inherit (and hold) it.
+exec 9>&- 2>/dev/null || true
+
 # -- Deferred bootstrap (background) ------------------------------------------
 # Heavy / network-dependent setup (apt packages, pip requirements, updater
 # install, splash-holder unit, splash precompute) runs detached in a transient

scripts/bs-bootstrap.sh

@@ -25,8 +25,8 @@ command -v feh >/dev/null 2>&1 || _missing="$_missing feh"
 command -v xsetroot >/dev/null 2>&1 || _missing="$_missing x11-xserver-utils"
 if [ -n "$_missing" ]; then
     # shellcheck disable=SC2086
-    apt-get install -y --quiet $_missing 2>/dev/null \
-        || echo "[bs-bootstrap] apt install deferred (offline?):$_missing"
+    apt-get -o DPkg::Lock::Timeout=60 install -y --quiet $_missing 2>/dev/null \
+        || echo "[bs-bootstrap] apt install deferred (offline or dpkg busy):$_missing"
 fi
 
 # 2. Python deps. PyYAML is required by the updater daemon's component loader.

scripts/bs-common.sh

@@ -6,18 +6,31 @@
 bs_migrate_moonraker_conf() {
     local conf="$1" tag="${2:-bs-common}" patched=false
     [ -f "$conf" ] || return 0
-    cp "$conf" "${conf}.bak" 2>/dev/null || true
+    # Back up only a sane-looking conf: never overwrite a good .bak with a corrupt one.
+    if grep -q '^\[server\]' "$conf" 2>/dev/null; then
+        cp "$conf" "${conf}.bak" 2>/dev/null || true
+    fi
     # Gate on the section existing: else the sed no-ops but patched=true restarts moonraker every boot.
     if ! grep -q "enable_system_updates" "$conf" && grep -q '^\[update_manager\]$' "$conf"; then
         sed -i '/^\[update_manager\]$/a enable_system_updates: False' "$conf"
         patched=true
         echo "[$tag] moonraker.conf: disabled system apt upgrades"
     fi
-    if grep -q "managed_services: klipper moonraker" "$conf"; then
+    # Scope checks to the BlocksScreen section so an unrelated match can't re-patch every boot.
+    local bs_um_section
+    bs_um_section=$(sed -n '/^\[update_manager BlocksScreen\]/,/^\[/p' "$conf" 2>/dev/null)
+    if printf '%s\n' "$bs_um_section" | grep -q "managed_services: klipper moonraker"; then
         sed -i '/^\[update_manager BlocksScreen\]/,/^\[/ s/managed_services: klipper moonraker/managed_services: BlocksScreen/' "$conf"
         patched=true
         echo "[$tag] moonraker.conf: fixed BlocksScreen managed_services"
     fi
+    # Old installers wrote primary_branch: master, which does not exist upstream;
+    # moonraker refuses to manage a repo whose primary branch is gone.
+    if printf '%s\n' "$bs_um_section" | grep -q "^primary_branch: master$"; then
+        sed -i '/^\[update_manager BlocksScreen\]/,/^\[/ s/^primary_branch: master$/primary_branch: main/' "$conf"
+        patched=true
+        echo "[$tag] moonraker.conf: fixed BlocksScreen primary_branch (master -> main)"
+    fi
     if ! grep -q "blocksscreen-single-owner" "$conf"; then
         bs_disable_overlapping_update_managers "$conf" "$tag" && patched=true
     fi
@@ -37,7 +50,15 @@ bs_ensure_spoolman_moonraker() {
     local spoolman_dir="${conf%/printer_data/config/moonraker.conf}/Spoolman"
     [ -d "$spoolman_dir/.venv" ] || return 1
     grep -q '^\[spoolman\]' "$conf" && return 1
-    printf '\n[spoolman]\nserver: localhost:7912\n' >>"$conf"
+    # Same-dir temp + atomic rename: a power cut mid-append could truncate the conf.
+    local tmp
+    tmp="$(mktemp -p "$(dirname "$conf")" .moonraker.conf.XXXXXX)" || return 1
+    if ! { cat "$conf" && printf '\n[spoolman]\nserver: localhost:7912\n'; } >"$tmp"; then
+        rm -f "$tmp"
+        return 1
+    fi
+    chmod --reference="$conf" "$tmp" 2>/dev/null || true
+    mv -f "$tmp" "$conf"
     echo "[$tag] moonraker.conf: added [spoolman] (server: localhost:7912)"
     return 0
 }
@@ -48,7 +69,7 @@ bs_disable_overlapping_update_managers() {
     local conf="$1" tag="${2:-bs-common}"
     [ -f "$conf" ] || return 1
     grep -q "blocksscreen-single-owner" "$conf" && return 1
-    local owned="RF50-Klipper happy-hare Klippain-ShakeTune mainsail-config crowsnest"
+    local owned="klipper RF50-Klipper happy-hare Klippain-ShakeTune mainsail-config crowsnest"
     local tmp
     # Same-dir temp + atomic rename: a power cut can never truncate moonraker.conf.
     tmp="$(mktemp -p "$(dirname "$conf")" .moonraker.conf.XXXXXX)" || return 1

scripts/bs-splash-holder.py

@@ -93,8 +93,13 @@ def _on_sigterm(_sig: int, _frame: object) -> None:
 
 signal.signal(signal.SIGTERM, _on_sigterm)
 
-# Switch to tty8 immediately - VT switch does not require fb0.
-_activate_tty8()
+# Switch to tty8 immediately - VT switch does not require fb0. Skip when X is
+# already up (first-install start lands mid-session): stealing the VT from the
+# live GUI on tty7 would look like a bricked screen until a power cycle.
+if os.path.exists("/tmp/.X11-unix/X0"):  # nosec B108 - canonical X11 socket path
+    _log("X session present - not activating tty8")
+else:
+    _activate_tty8()
 
 # Clear tty8 and hide the cursor (no bare text): the screen stays black until the
 # splash image is written to fb0 below, so the user only ever sees the logo splash.

scripts/install-updater.sh

@@ -20,13 +20,22 @@ _BSENV_USER="blocks"
 _BSENV_HOME=$(getent passwd "$_BSENV_USER" | cut -d: -f6)
 BSENV="${BLOCKSSCREEN_VENV:-${_BSENV_HOME}/.BlocksScreen-env}"
 
+# Venv-mutating commands run as blocks: root-owned dists break later pip-as-blocks runs.
+_as_blocks() { if [ "$(id -u)" = "0" ]; then runuser -u "$_BSENV_USER" -- "$@"; else "$@"; fi; }
+# Atomic root install: a power cut must never leave a truncated-but-present file
+# (the [ -f ] self-heal guards would then never rewrite it).
+_install_atomic() {
+    local mode="$1" src="$2" dst="$3"
+    sudo install -m "$mode" "$src" "${dst}.new" && sudo mv -Tf "${dst}.new" "$dst"
+}
+
 echo_info "Installing D-Bus policy ..."
-sudo cp "$SCRIPT_PATH/com.blockscreen.Updater.conf" /etc/dbus-1/system.d/
+_install_atomic 0644 "$SCRIPT_PATH/com.blockscreen.Updater.conf" /etc/dbus-1/system.d/com.blockscreen.Updater.conf
 sudo systemctl reload dbus || true
 echo_ok "D-Bus policy installed"
 
 echo_info "Installing D-Bus activation file ..."
-sudo cp "$SCRIPT_PATH/com.blockscreen.Updater.service" /usr/share/dbus-1/system-services/
+_install_atomic 0644 "$SCRIPT_PATH/com.blockscreen.Updater.service" /usr/share/dbus-1/system-services/com.blockscreen.Updater.service
 echo_ok "D-Bus activation file installed"
 
 echo_info "Installing bs-apt-helper (root-owned apt wrapper) ..."
@@ -40,7 +49,8 @@ SERVICE=$(cat "$SCRIPT_PATH/BlocksScreen-updater.service")
 SERVICE=${SERVICE//BS_DIR/$BS_PATH}
 SERVICE=${SERVICE//BSENV/$BSENV}
 SERVICE=${SERVICE//BS_PRIMARY_HOME/$_BSENV_HOME}
-echo "$SERVICE" | sudo tee /etc/systemd/system/BlocksScreen-updater.service >/dev/null
+echo "$SERVICE" | sudo tee /etc/systemd/system/.BlocksScreen-updater.service.new >/dev/null
+sudo mv -Tf /etc/systemd/system/.BlocksScreen-updater.service.new /etc/systemd/system/BlocksScreen-updater.service
 sudo systemctl daemon-reload
 sudo systemctl enable BlocksScreen-updater.service
 sudo systemctl restart BlocksScreen-updater.service || true
@@ -49,7 +59,8 @@ echo_ok "BlocksScreen-updater service installed and started"
 echo_info "Installing Spoolman service unit (inactive until the updater provisions it) ..."
 SPOOLMAN_SVC=$(cat "$SCRIPT_PATH/Spoolman.service")
 SPOOLMAN_SVC=${SPOOLMAN_SVC//BS_PRIMARY_HOME/$_BSENV_HOME}
-echo "$SPOOLMAN_SVC" | sudo tee /etc/systemd/system/Spoolman.service >/dev/null
+echo "$SPOOLMAN_SVC" | sudo tee /etc/systemd/system/.Spoolman.service.new >/dev/null
+sudo mv -Tf /etc/systemd/system/.Spoolman.service.new /etc/systemd/system/Spoolman.service
 sudo systemctl daemon-reload
 echo_ok "Spoolman service unit installed"
 
@@ -81,8 +92,10 @@ else
 fi
 # Daemon self-restart target; never in components.yaml.
 _emit_svc_rules BlocksScreen-updater.service
-# Spoolman is provisioned on demand; enable rule needed for its first clean start.
+# Spoolman is provisioned on demand; enable rules needed for its first clean start
+# (hooks/Spoolman.sh uses `enable --now`; sudoers args must match exactly).
 printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl enable Spoolman.service\n' >>"$SUDOERS_TMP"
+printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl enable --now Spoolman.service\n' >>"$SUDOERS_TMP"
 if sudo visudo -cf "$SUDOERS_TMP" >/dev/null 2>&1; then
     sudo install -m 0440 "$SUDOERS_TMP" "$SUDOERS_FILE"
     echo_ok "Sudoers rules installed"
@@ -179,10 +192,12 @@ echo_info "Installing BlocksScreen-deploy path unit (sudo-free hook trigger) ...
 DEPLOY_SVC=$(cat "$SCRIPT_PATH/BlocksScreen-deploy.service")
 DEPLOY_SVC=${DEPLOY_SVC//BS_DIR/$BS_PATH}
 DEPLOY_SVC=${DEPLOY_SVC//BS_PRIMARY_HOME/$_BSENV_HOME}
-echo "$DEPLOY_SVC" | sudo tee /etc/systemd/system/BlocksScreen-deploy.service >/dev/null
+echo "$DEPLOY_SVC" | sudo tee /etc/systemd/system/.BlocksScreen-deploy.service.new >/dev/null
+sudo mv -Tf /etc/systemd/system/.BlocksScreen-deploy.service.new /etc/systemd/system/BlocksScreen-deploy.service
 DEPLOY_PATH=$(cat "$SCRIPT_PATH/BlocksScreen-deploy.path")
 DEPLOY_PATH=${DEPLOY_PATH//BS_PRIMARY_HOME/$_BSENV_HOME}
-echo "$DEPLOY_PATH" | sudo tee /etc/systemd/system/BlocksScreen-deploy.path >/dev/null
+echo "$DEPLOY_PATH" | sudo tee /etc/systemd/system/.BlocksScreen-deploy.path.new >/dev/null
+sudo mv -Tf /etc/systemd/system/.BlocksScreen-deploy.path.new /etc/systemd/system/BlocksScreen-deploy.path
 sudo systemctl daemon-reload
 sudo systemctl enable --now BlocksScreen-deploy.path
 echo_ok "BlocksScreen-deploy path unit installed"
@@ -207,23 +222,23 @@ echo_ok "post-merge hook installed"
 
 echo_info "Installing Python requirements ..."
 # Best-effort pip self-update; must not block the requirements install below.
-"$BSENV/bin/pip" install --quiet --upgrade pip 2>/dev/null || true
-apt-get install -y --quiet libsystemd-dev python3-dev 2>/dev/null || true
+_as_blocks "$BSENV/bin/pip" install --quiet --upgrade pip 2>/dev/null || true
+sudo apt-get -o DPkg::Lock::Timeout=60 install -y --quiet libsystemd-dev python3-dev 2>/dev/null || true
 # xsetroot is used as belt-and-suspenders cursor hiding alongside the Xorg -nocursor server flag.
-sudo apt-get install -y --quiet x11-xserver-utils 2>/dev/null || true
+sudo apt-get -o DPkg::Lock::Timeout=60 install -y --quiet x11-xserver-utils 2>/dev/null || true
 # sdbus pinned; --no-binary needs libsystemd-dev; only-if-needed skips satisfied.
-"$BSENV/bin/pip" install --quiet --only-binary :all: --no-binary sdbus,sdbus-networkmanager \
+_as_blocks "$BSENV/bin/pip" install --quiet --only-binary :all: --no-binary sdbus,sdbus-networkmanager \
     --upgrade-strategy=only-if-needed \
     -r "$BS_PATH/scripts/requirements.txt" || true
 echo_ok "Python requirements installed"
 
 # uv: Spoolman's dependency installer (run-from-source); provide it for the hook.
 echo_info "Ensuring uv is available for Spoolman provisioning ..."
-"$BSENV/bin/pip" install --quiet --upgrade uv 2>/dev/null || true
+_as_blocks "$BSENV/bin/pip" install --quiet --upgrade uv 2>/dev/null || true
 echo_ok "uv ready"
 
 echo_info "Pre-generating framebuffer splash cache ..."
-"$BSENV/bin/python3.11" "$SCRIPT_PATH/bs-splash.py" --precompute 2>/dev/null || true
+_as_blocks "$BSENV/bin/python3.11" "$SCRIPT_PATH/bs-splash.py" --precompute 2>/dev/null || true
 echo_ok "Splash cache ready"
 
 echo_ok "BlocksScreen updater setup complete"

scripts/install.sh

@@ -413,11 +413,11 @@ function add_moonraker_update() {
 
 [update_manager BlocksScreen]
 type: git_repo
-primary_branch: master
+primary_branch: main
 path: $BS_PATH
 origin: https://github.com/BlocksTechnology/BlocksScreen.git
 is_system_service: True
-managed_services: klipper moonraker
+managed_services: BlocksScreen
 virtualenv: $BSENV
 requirements: scripts/requirements.txt
 system_dependencies: scripts/system-dependencies.json

scripts/post-merge

@@ -8,6 +8,10 @@ _BSENV_USER="blocks"
 _BSENV_HOME=$(getent passwd "$_BSENV_USER" | cut -d: -f6)
 BSENV="${BLOCKSSCREEN_VENV:-${_BSENV_HOME}/.BlocksScreen-env}"
 
+# A root pull must not leave root-owned dists in the blocks venv (breaks later pip runs).
+_as_blocks() { if [ "$(id -u)" = "0" ]; then runuser -u "$_BSENV_USER" -- "$@"; else "$@"; fi; }
+_own() { [ "$(id -u)" = "0" ] && chown "$_BSENV_USER" "$1" 2>/dev/null || true; }
+
 # shellcheck source=/dev/null
 if [ -f "$SCRIPT_PATH/bs-common.sh" ]; then . "$SCRIPT_PATH/bs-common.sh"; fi
 
@@ -23,12 +27,13 @@ SENTINEL="$BSENV/.blockscreen-reqs-hash"
 ATTEMPT="$BSENV/.blockscreen-reqs-attempt"
 if [ ! -f "$SENTINEL" ] || [ "$(cat "$SENTINEL")" != "$REQS_HASH" ]; then
     echo "[post-merge] requirements changed - pre-installing deps (this may take a few minutes) ..."
-    if "$BSENV/bin/pip" install --quiet \
+    if _as_blocks "$BSENV/bin/pip" install --quiet \
         --only-binary :all: \
         --no-binary sdbus,sdbus-networkmanager \
         --upgrade-strategy=only-if-needed \
         -r "$BS_PATH/scripts/requirements.txt"; then
         echo "$REQS_HASH" >"$SENTINEL"
+        _own "$SENTINEL"
         rm -f "$ATTEMPT" 2>/dev/null || true
         echo "[post-merge] deps installed"
     else
@@ -38,9 +43,11 @@ if [ ! -f "$SENTINEL" ] || [ "$(cat "$SENTINEL")" != "$REQS_HASH" ]; then
         case "$_cnt" in '' | *[!0-9]*) _cnt=0 ;; esac
         if [ "$_prev" = "$REQS_HASH" ]; then _cnt=$((_cnt + 1)); else _cnt=1; fi
         echo "$REQS_HASH $_cnt" >"$ATTEMPT"
+        _own "$ATTEMPT"
         if [ "$_cnt" -ge 3 ]; then
             echo "[post-merge] deps failed ${_cnt}x; giving up to stop thrash (venv left as-is)"
             echo "$REQS_HASH" >"$SENTINEL"
+            _own "$SENTINEL"
         fi
     fi
 fi
@@ -55,7 +62,7 @@ if [ ! -f /etc/systemd/system/BlocksScreen-updater.service ]; then
 fi
 
 # 4. Precompute splash cache so the first Qt restart shows the right logo.
-"$BSENV/bin/python3.11" "$SCRIPT_PATH/bs-splash.py" --precompute 2>/dev/null || true
+_as_blocks "$BSENV/bin/python3.11" "$SCRIPT_PATH/bs-splash.py" --precompute 2>/dev/null || true
 
 
 # Under the daemon's own pull a restart here aborts the batch: defer via sentinel.

tests/scripts/test_moonraker_ownership.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import subprocess
 from pathlib import Path
 
@@ -14,6 +15,10 @@
 [update_manager mainsail]
 type: web
 
+[update_manager klipper]
+type: git_repo
+path: ~/klipper
+
 [update_manager RF50-Klipper]
 type: git_repo
 path: ~/RF50-Klipper
@@ -44,6 +49,7 @@ def test_owned_blocks_commented_others_untouched(tmp_path: Path) -> None:
     conf = tmp_path / "moonraker.conf"
     conf.write_text(_CONF)
     out = _run(conf)
+    assert "#[update_manager klipper]" in out  # updater owns klipper, not moonraker
     assert "#[update_manager RF50-Klipper]" in out
     assert "#[update_manager crowsnest]" in out
     assert "[update_manager mainsail]" in out  # web client kept
@@ -57,3 +63,65 @@ def test_idempotent(tmp_path: Path) -> None:
     first = _run(conf)
     second = _run(conf)
     assert first == second  # no double-comment on re-run
+
+
+_MIGRATE_CONF = """\
+[server]
+host: 0.0.0.0
+
+[update_manager]
+enable_system_updates: False
+
+[update_manager BlocksScreen]
+type: git_repo
+primary_branch: master
+managed_services: klipper moonraker
+
+[update_manager other]
+primary_branch: master
+"""
+
+
+def _migrate(conf: Path) -> str:
+    # Shim sudo: the function ends with `sudo systemctl restart moonraker`; real
+    # sudo would prompt on a dev TTY (test hang) or bounce services on the printer.
+    shim = conf.parent / "bin"
+    shim.mkdir(exist_ok=True)
+    fake_sudo = shim / "sudo"
+    fake_sudo.write_text("#!/bin/sh\nexit 0\n")
+    fake_sudo.chmod(0o755)
+    env = {**os.environ, "PATH": f"{shim}:{os.environ.get('PATH', '')}"}
+    subprocess.run(
+        [
+            "bash",
+            "-c",
+            f'. "{_FN}"; bs_migrate_moonraker_conf "{conf}" test',
+        ],
+        check=False,
+        capture_output=True,
+        text=True,
+        env=env,
+        timeout=20,
+    )
+    return conf.read_text()
+
+
+def test_migrate_fixes_blocksscreen_branch_and_services(tmp_path: Path) -> None:
+    conf = tmp_path / "moonraker.conf"
+    conf.write_text(_MIGRATE_CONF)
+    out = _migrate(conf)
+    bs = out.split("[update_manager BlocksScreen]")[1].split("[update_manager other]")[
+        0
+    ]
+    assert "primary_branch: main" in bs  # origin/master does not exist upstream
+    assert "managed_services: BlocksScreen" in bs
+    # Patches are scoped to the BlocksScreen section: others keep their values.
+    assert "primary_branch: master" in out.split("[update_manager other]")[1]
+
+
+def test_migrate_is_idempotent(tmp_path: Path) -> None:
+    conf = tmp_path / "moonraker.conf"
+    conf.write_text(_MIGRATE_CONF)
+    first = _migrate(conf)
+    second = _migrate(conf)
+    assert first == second