updater: security hardening, apt rollback, cancel button, elapsed timer, error toasts, +15 tests

Author: gmmcosta15

BlocksScreen/lib/panels/mainWindow.py

@@ -236,11 +236,17 @@ def __init__(self):
         self.ws.connected_signal.connect(self._on_moonraker_connected_post_update)
         self.update_page.request_update.connect(self.updater_worker.trigger_update)
         self.update_page.request_status.connect(self.updater_worker.trigger_status)
+        self.update_page.request_cancel.connect(self.updater_worker.trigger_cancel)
         self.update_page.update_available.connect(self.on_update_available)
         self.update_page.call_load_panel.connect(self.show_LoadScreen)
         self.update_page.disable_popups.connect(self.popup_toggle)
         self.update_page.update_back_btn.clicked.connect(self.update_page.hide)
         self.updater_worker.step_complete.connect(self.update_page.handle_step_complete)
+        self.updater_worker.error_occurred.connect(
+            self.update_page.handle_error_occurred
+        )
+        self.updater_worker.rollback_done.connect(self.update_page.handle_rollback_done)
+        self.updater_worker.recover_done.connect(self.update_page.handle_recover_done)
         self.ws.klippy_state_signal.connect(self._on_klippy_state)
         self.utilitiesPanel.show_update_page.connect(self.show_update_page)
         self.conn_window.update_button_clicked.connect(self.show_update_page)

BlocksScreen/lib/panels/widgets/updatePage.py

@@ -25,6 +25,9 @@ class UpdatePage(QtWidgets.QWidget):
     request_status: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal(
         name="request-status"
     )
+    request_cancel: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal(
+        name="request-cancel"
+    )
     update_available: typing.ClassVar[QtCore.pyqtSignal] = QtCore.pyqtSignal(
         bool, name="update-available"
     )
@@ -63,17 +66,44 @@ def __init__(self) -> None:
         self._busy: bool = False
         self._update_avail: bool = False
         self._post_update_status_pending: bool = False
+        self._elapsed_time_seconds: int = 0
+        self._elapsed_timer: QtCore.QTimer = QtCore.QTimer(self)
+        self._elapsed_timer.setSingleShot(False)
+        self._elapsed_timer.setInterval(1000)
+        self._elapsed_timer.timeout.connect(self._update_elapsed_time)
         self.show_loading(True)
 
     def _request_status_debounced(self) -> None:
         self._status_debounce.start(500)
 
+    def _update_elapsed_time(self) -> None:
+        """Update and display elapsed time counter."""
+        self._elapsed_time_seconds += 1
+        minutes = self._elapsed_time_seconds // 60
+        seconds = self._elapsed_time_seconds % 60
+        self._elapsed_time_label.setText(f"{minutes:02d}:{seconds:02d}")
+
     def showEvent(self, a0: QtGui.QShowEvent | None) -> None:
         self.build_cards()
         self._post_update_status_pending = True
         self._request_status_debounced()
         return super().showEvent(a0)
 
+    def resizeEvent(self, a0: QtGui.QResizeEvent | None) -> None:
+        """Position elapsed time label and cancel button within the load widget area."""
+        if self._loadwidget.isVisible():
+            load_widget_height = self._loadwidget.height()
+            load_widget_width = self._loadwidget.width()
+            # Position elapsed time label below the spinner (around 70% down)
+            elapsed_y = int(load_widget_height * 0.70)
+            elapsed_x = (load_widget_width - 100) // 2
+            self._elapsed_time_label.setGeometry(elapsed_x, elapsed_y, 100, 40)
+            # Position cancel button below the elapsed time label
+            cancel_y = int(load_widget_height * 0.80)
+            cancel_x = (load_widget_width - 240) // 2
+            self._cancel_btn.setGeometry(cancel_x, cancel_y, 240, 50)
+        return super().resizeEvent(a0)
+
     def _needs_update(self, status: ComponentStatus) -> bool:
         return bool(
             status.commits_behind
@@ -253,7 +283,15 @@ def handle_busy_changed(self, busy: bool) -> None:
         _log.info("handle_busy_changed: %s", busy)
         self._busy = busy
         self.show_loading(busy)
-        if not busy:
+        if busy:
+            self._elapsed_time_seconds = 0
+            self._elapsed_timer.start()
+            self._elapsed_time_label.show()
+            self._cancel_btn.show()
+        else:
+            self._elapsed_timer.stop()
+            self._elapsed_time_label.hide()
+            self._cancel_btn.hide()
             self._post_update_status_pending = True
             self._request_status_debounced()
             # Don't dismiss the overlay yet — wait for status_ready to refresh
@@ -263,6 +301,12 @@ def handle_busy_changed(self, busy: bool) -> None:
     def on_update_all_clicked(self) -> None:
         self._do_update()
 
+    @QtCore.pyqtSlot(name="on-cancel-clicked")
+    def _on_cancel_clicked(self) -> None:
+        """Emit cancel signal when user clicks cancel button."""
+        _log.info("Cancel button clicked")
+        self.request_cancel.emit()
+
     @QtCore.pyqtSlot(name="do-update")
     def _do_update(self) -> None:
         self.request_update.emit("")
@@ -280,6 +324,30 @@ def handle_step_complete(self, name: str, step: int, total: int) -> None:
         _log.info("step_complete: %s %d/%d (%s)", name, step, total, label)
         self.call_load_panel.emit(True, f"{name}: {label} ({step}/{total})")
 
+    def _show_toast(self, message: str, *, success: bool = False) -> None:
+        color = "#4caf50" if success else "#ef5350"
+        self._toast.setStyleSheet(
+            f"background: {color}; color: #fff; padding: 4px 12px; border-radius: 8px;"
+        )
+        self._toast.setText(message)
+        self._toast.show()
+        self._toast.raise_()
+        self._toast_timer.start()
+
+    def handle_error_occurred(self, name: str, reason: str) -> None:
+        self._show_toast(f"{name}: {reason}")
+
+    def handle_rollback_done(self, name: str, success: bool) -> None:
+        self._show_toast(
+            f"{name}: {'rolled back' if success else 'rollback failed'}",
+            success=success,
+        )
+
+    def handle_recover_done(self, name: str, success: bool) -> None:
+        self._show_toast(
+            f"{name}: recovery {'complete' if success else 'failed'}", success=success
+        )
+
     def show_loading(self, loading: bool = False) -> None:
         self.setUpdatesEnabled(False)
         self._loadwidget.setVisible(loading)
@@ -373,12 +441,31 @@ def _setupUI(self) -> None:
         self._scroll_area.setWidget(self._scroll_content)
         content.addWidget(self._scroll_area, 1)
 
-        # Loading overlay
+        # Loading overlay with elapsed time and cancel button
         self._loadwidget = LoadingOverlayWidget(
             self, LoadingOverlayWidget.AnimationGIF.DEFAULT
         )
         content.addWidget(self._loadwidget, 1)
 
+        # Elapsed time label (positioned in the load widget area via custom overlay)
+        self._elapsed_time_label = QtWidgets.QLabel("00:00", self._loadwidget)
+        self._elapsed_time_label.setStyleSheet(
+            "color: rgba(255, 255, 255, 200); background: transparent;"
+        )
+        self._elapsed_time_label.setFont(QtGui.QFont(self._font_family, 16))
+        self._elapsed_time_label.setAlignment(QtCore.Qt.AlignmentFlag.AlignCenter)
+        self._elapsed_time_label.setFixedWidth(100)
+        self._elapsed_time_label.hide()
+
+        # Cancel button (positioned in the load widget area)
+        self._cancel_btn = BlocksCustomButton(self._loadwidget)
+        self._cancel_btn.setMinimumSize(QtCore.QSize(180, 50))
+        self._cancel_btn.setMaximumSize(QtCore.QSize(240, 50))
+        self._cancel_btn.setFont(QtGui.QFont(self._font_family, 18))
+        self._cancel_btn.setText("Cancel")
+        self._cancel_btn.clicked.connect(self._on_cancel_clicked)
+        self._cancel_btn.hide()
+
         # Update All button
         self.update_all_btn = BlocksCustomButton(self)
         self.update_all_btn.setMinimumSize(QtCore.QSize(240, 70))
@@ -391,6 +478,18 @@ def _setupUI(self) -> None:
         content.addWidget(self.update_all_btn, 0, QtCore.Qt.AlignmentFlag.AlignCenter)
         self.update_all_btn.hide()
 
+        self._toast = QtWidgets.QLabel(self)
+        self._toast.setWordWrap(True)
+        self._toast.setAlignment(QtCore.Qt.AlignmentFlag.AlignCenter)
+        self._toast.setFont(QtGui.QFont(self._font_family, 13))
+        self._toast.setFixedHeight(46)
+        self._toast.hide()
+        content.addWidget(self._toast, 0)
+        self._toast_timer = QtCore.QTimer(self)
+        self._toast_timer.setSingleShot(True)
+        self._toast_timer.setInterval(8000)
+        self._toast_timer.timeout.connect(self._toast.hide)
+
         _arrow = QtGui.QPixmap(":/arrow_icons/media/btn_icons/arrow_right.svg")
         self._chevron_right: QtGui.QPixmap = _arrow
         self._chevron_down: QtGui.QPixmap = _arrow.transformed(

BlocksScreen/lib/updater_worker.py

@@ -105,9 +105,12 @@ async def _async_initialize(self) -> None:
             self._listener_tasks.append(task)
             task.add_done_callback(self._on_listener_done)
 
-        # Yield so listener tasks enter their async-for loops (subscribe to signals)
-        # before we poll current state, avoiding a missed busy_changed(False) on reconnect.
-        await asyncio.sleep(0)
+        # Give each listener task a scheduling slot to enter its async-for loop
+        # (and register its D-Bus signal subscription) before we poll current state.
+        # One sleep(0) per task is sufficient: asyncio runs all ready callbacks
+        # before yielding back to us.
+        for _ in listeners:
+            await asyncio.sleep(0)
 
         try:
             busy = await self._proxy.get_busy()
@@ -305,12 +308,15 @@ async def _listen_busy_changed(self) -> None:
             self.busy_changed.emit(busy)
 
     async def _busy_watchdog(self) -> None:
-        """Emit daemon_unavailable if BusyChanged(False) does not arrive within 60 seconds."""  # noqa: E501
+        """Emit daemon_unavailable if BusyChanged(False) does not arrive within 360 seconds.
+
+        360s = 300s apt-get upgrade hard deadline + 30s systemctl + 30s buffer.
+        """
         if self._busy_false_event is None:
             _msg = "_busy_false_event not initialized"
             raise RuntimeError(_msg)
         try:
-            async with asyncio.timeout(60.0):
+            async with asyncio.timeout(360.0):
                 await self._busy_false_event.wait()
         except TimeoutError:
             _log.error("busy watchdog timed out, daemon is unavailable")

scripts/BlocksScreen-updater.service

@@ -13,5 +13,21 @@ ExecStart=BSENV/bin/python3.11 -m updater daemon
 Restart=always
 RestartSec=5
 
+# Security hardening
+NoNewPrivileges=yes
+PrivateTmp=yes
+ProtectKernelTunables=yes
+ProtectKernelModules=yes
+ProtectControlGroups=yes
+RestrictRealtime=yes
+LockPersonality=yes
+MemoryDenyWriteExecute=yes
+RestrictNamespaces=yes
+RemoveIPC=yes
+ProtectHostname=yes
+RestrictSUIDSGID=yes
+SystemCallFilter=@system-service
+SystemCallErrorNumber=EPERM
+
 [Install]
 WantedBy=multi-user.target

scripts/install-updater.sh

@@ -40,9 +40,19 @@ echo_info "Installing sudoers rules for updater ..."
 SUDOERS_FILE="/etc/sudoers.d/blockscreen-updater"
 SUDOERS_TMP=$(mktemp)
 printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/apt-get update\n' >>"$SUDOERS_TMP"
-printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/apt-get upgrade -y\n' >>"$SUDOERS_TMP"
-printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl restart *\n' >>"$SUDOERS_TMP"
+printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/apt-get install --only-upgrade -y *\n' >>"$SUDOERS_TMP"
 printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl reboot\n' >>"$SUDOERS_TMP"
+# Derive allowed restart targets from components.yaml instead of wildcard
+_COMP_YAML="$BS_PATH/updater/components.yaml"
+if [[ -f "$_COMP_YAML" ]]; then
+    while IFS= read -r _svc; do
+        printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl restart %s\n' "$_svc" >>"$SUDOERS_TMP"
+    done < <(grep '^\s*service:' "$_COMP_YAML" | awk '{print $2}' | sort -u)
+else
+    for _svc in klipper.service moonraker.service crowsnest.service KlipperScreen.service BlocksScreen.service; do
+        printf 'blocks ALL=(ALL) NOPASSWD: /usr/bin/systemctl restart %s\n' "$_svc" >>"$SUDOERS_TMP"
+    done
+fi
 if sudo visudo -cf "$SUDOERS_TMP" >/dev/null 2>&1; then
     sudo install -m 0440 "$SUDOERS_TMP" "$SUDOERS_FILE"
     echo_ok "Sudoers rules installed"

tests/updater/test_components_unit.py

@@ -3,7 +3,7 @@
 import sys
 import logging
 from pathlib import Path
-from unittest.mock import patch, mock_open
+from unittest.mock import patch, mock_open, MagicMock
 
 from updater.components import load_components
 
@@ -91,10 +91,17 @@ def test_override_permission_error_skipped(self, caplog):
 
 
 class TestYamlMerge:
+    def _safe_stat_mock(self):
+        """Return a mock stat result for safe (owner-only) file permissions."""
+        mock_stat = MagicMock()
+        mock_stat.st_mode = 0o100600  # Regular file, owner-only
+        return mock_stat
+
     def test_overrides_path_by_name(self):
         with (
             patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
             patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=self._safe_stat_mock()),
             patch("pathlib.Path.is_dir", return_value=True),
         ):
             components, poll = load_components()
@@ -106,6 +113,7 @@ def test_appends_new_component(self):
         with (
             patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
             patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=self._safe_stat_mock()),
             patch("pathlib.Path.is_dir", return_value=True),
         ):
             components, poll = load_components()
@@ -115,6 +123,7 @@ def test_keeps_bundled_fields_not_in_override(self):
         with (
             patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
             patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=self._safe_stat_mock()),
             patch("pathlib.Path.is_dir", return_value=True),
         ):
             components, poll = load_components()
@@ -127,6 +136,7 @@ def test_syntax_error_falls_back_to_bundled(self, caplog):
         with (
             patch("builtins.open", _mock_load(BUNDLE_YAML, invalid_yaml)),
             patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=self._safe_stat_mock()),
             patch("pathlib.Path.is_dir", return_value=True),
             caplog.at_level(logging.ERROR, logger="updater.components"),
         ):
@@ -270,3 +280,65 @@ def test_yaml_missing_returns_empty_list(self, caplog):
         assert components == []
         assert poll == 1440 * 60.0  # default poll interval
         assert any("not installed" in r.message for r in caplog.records)
+
+
+class TestOverridePermissions:
+    def test_override_skipped_when_world_writable(self, caplog):
+        """Override file with mode 0o622 (world-writable) is skipped."""
+        mock_stat = MagicMock()
+        mock_stat.st_mode = 0o100622  # Regular file, world-writable
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=mock_stat),
+            patch("pathlib.Path.is_dir", return_value=True),
+            caplog.at_level(logging.WARNING, logger="updater.components"),
+        ):
+            components, poll = load_components()
+        # Should only have bundled components, no override merged
+        names = [c.name for c in components]
+        assert "klipper" in names
+        assert "blockscreen" in names
+        assert any("writable by group/others" in r.message for r in caplog.records)
+
+    def test_override_skipped_when_group_writable(self, caplog):
+        """Override file with mode 0o620 (group-writable) is skipped."""
+        mock_stat = MagicMock()
+        mock_stat.st_mode = 0o100620  # Regular file, group-writable
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=mock_stat),
+            patch("pathlib.Path.is_dir", return_value=True),
+            caplog.at_level(logging.WARNING, logger="updater.components"),
+        ):
+            components, poll = load_components()
+        assert any("writable by group/others" in r.message for r in caplog.records)
+
+    def test_override_loaded_with_safe_permissions(self):
+        """Override file with mode 0o600 (owner-only) is loaded."""
+        mock_stat = MagicMock()
+        mock_stat.st_mode = 0o100600  # Regular file, owner-only
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", return_value=mock_stat),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
+            components, poll = load_components()
+        # Should have merged components from override
+        assert any(c.name == "my-plugin" for c in components)
+        klipper = next(c for c in components if c.name == "klipper")
+        assert str(klipper.path).endswith("costum_klipper")
+
+    def test_override_skipped_when_stat_fails(self, caplog):
+        """Override file that cannot be stat'd is skipped."""
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.stat", side_effect=OSError("Permission denied")),
+            patch("pathlib.Path.is_dir", return_value=True),
+            caplog.at_level(logging.WARNING, logger="updater.components"),
+        ):
+            components, poll = load_components()
+        assert any("Cannot stat override path" in r.message for r in caplog.records)