From c84e269825aa460d468816626e46d6fc23667a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 3 Oct 2024 16:40:34 +0200 Subject: [PATCH] Catch installation errors and display them. Fixes #193 --- dangerzone/gui/main_window.py | 76 ++++++++++++------ dangerzone/isolation_provider/container.py | 20 +++-- dangerzone/util.py | 11 +++ poetry.lock | 21 ++++- pyproject.toml | 1 + tests/gui/test_main_window.py | 93 +++++++++++++++++++++- tests/isolation_provider/base.py | 1 - 7 files changed, 191 insertions(+), 32 deletions(-) diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index 43fb4c8f0..8eed6fe06 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -20,15 +20,20 @@ from PySide6.QtWidgets import QTextEdit except ImportError: from PySide2 import QtCore, QtGui, QtSvg, QtWidgets - from PySide2.QtWidgets import QAction, QTextEdit from PySide2.QtCore import Qt + from PySide2.QtWidgets import QAction, QTextEdit from .. import errors from ..document import SAFE_EXTENSION, Document from ..isolation_provider.container import Container, NoContainerTechException from ..isolation_provider.dummy import Dummy from ..isolation_provider.qubes import Qubes, is_qubes_native_conversion -from ..util import get_resource_path, get_subprocess_startupinfo, get_version +from ..util import ( + format_exception, + get_resource_path, + get_subprocess_startupinfo, + get_version, +) from .logic import Alert, CollapsibleBox, DangerzoneGui, UpdateDialog from .updater import UpdateReport @@ -388,15 +393,24 @@ def closeEvent(self, e: QtGui.QCloseEvent) -> None: class InstallContainerThread(QtCore.QThread): - finished = QtCore.Signal() + finished = QtCore.Signal(str) def __init__(self, dangerzone: DangerzoneGui) -> None: super(InstallContainerThread, self).__init__() self.dangerzone = dangerzone def run(self) -> None: - self.dangerzone.isolation_provider.install() - self.finished.emit() + error = None + try: + installed = self.dangerzone.isolation_provider.install() + except Exception as e: + log.error("Container installation problem") + error = format_exception(e) + else: + if not installed: + error = "The image cannot be found. This can be caused by a faulty container image." + finally: + self.finished.emit(error) class WaitingWidget(QtWidgets.QWidget): @@ -423,9 +437,10 @@ def __init__(self) -> None: # Enable copying self.setTextInteractionFlags(Qt.TextSelectableByMouse) - def set_content(self, error: str) -> None: - self.setPlainText(error) - self.setVisible(True) + def set_content(self, error: Optional[str] = None) -> None: + if error: + self.setPlainText(error) + self.setVisible(True) class WaitingWidgetContainer(WaitingWidget): @@ -438,7 +453,6 @@ class WaitingWidgetContainer(WaitingWidget): # # Linux states # - "install_container" - finished = QtCore.Signal() def __init__(self, dangerzone: DangerzoneGui) -> None: super(WaitingWidgetContainer, self).__init__() @@ -509,20 +523,43 @@ def check_state(self) -> None: # Update the state self.state_change(state, error) + def show_error(self, msg: str, details: Optional[str] = None) -> None: + self.label.setText(msg) + show_traceback = details is not None + if show_traceback: + self.traceback.set_content(details) + self.traceback.setVisible(show_traceback) + self.buttons.show() + + def show_message(self, msg: str) -> None: + self.label.setText(msg) + self.traceback.setVisible(False) + self.buttons.hide() + + def installation_finished(self, error: Optional[str] = None) -> None: + if error: + msg = ( + "During installation of the dangerzone image,
" + "the following error occured:" + ) + self.show_error(msg, error) + else: + self.finished.emit() + def state_change(self, state: str, error: Optional[str] = None) -> None: if state == "not_installed": if platform.system() == "Linux": - self.label.setText( + self.show_error( "Dangerzone requires Podman

" "Install it and retry." ) else: - self.label.setText( + self.show_error( "Dangerzone requires Docker Desktop

" "Download Docker Desktop" ", install it, and open it." ) - self.buttons.show() + elif state == "not_running": if platform.system() == "Linux": # "not_running" here means that the `podman image ls` command failed. @@ -530,27 +567,20 @@ def state_change(self, state: str, error: Optional[str] = None) -> None: "Dangerzone requires Podman

" "Podman is installed but cannot run properly. See errors below" ) - if error: - self.traceback.set_content(error) - - self.label.setText(message) - else: - self.label.setText( + message = ( "Dangerzone requires Docker Desktop

" "Docker is installed but isn't running.

" "Open Docker and make sure it's running in the background." ) - self.buttons.show() + self.show_error(message, error) else: - self.label.setText( + self.show_message( "Installing the Dangerzone container image.

" "This might take a few minutes..." ) - self.buttons.hide() - self.traceback.setVisible(False) self.install_container_t = InstallContainerThread(self.dangerzone) - self.install_container_t.finished.connect(self.finished) + self.install_container_t.finished.connect(self.installation_finished) self.install_container_t.start() diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index fe6626c17..0a5c2b149 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -165,7 +165,7 @@ def install() -> bool: startupinfo=get_subprocess_startupinfo(), ) - chunk_size = 10240 + chunk_size = 4 << 20 compressed_container_path = get_resource_path("container.tar.gz") with gzip.open(compressed_container_path) as f: while True: @@ -175,19 +175,20 @@ def install() -> bool: p.stdin.write(chunk) else: break - p.communicate() + _, err = p.communicate() + if p.returncode < 0: + raise RuntimeError(f"Could not install container image: {err.decode()}") - if not Container.is_container_installed(): - log.error("Failed to install the container image") + if not Container.is_container_installed(raise_on_error=True): return False log.info("Container image installed") return True @staticmethod - def is_container_installed() -> bool: + def is_container_installed(raise_on_error: bool = False) -> bool: """ - See if the podman container is installed. Linux only. + See if the container is installed. """ # Get the image id with open(get_resource_path("image-id.txt")) as f: @@ -214,6 +215,13 @@ def is_container_installed() -> bool: elif found_image_id == "": pass else: + msg = ( + f"{Container.CONTAINER_NAME} images found, but IDs do not match." + f"Found: {found_image_id}, Expected: {','.join(expected_image_ids)}" + ) + if raise_on_error: + raise RuntimeError(msg) + log.info(msg) log.info("Deleting old dangerzone container image") try: diff --git a/dangerzone/util.py b/dangerzone/util.py index 311288c7a..5bd9aedba 100644 --- a/dangerzone/util.py +++ b/dangerzone/util.py @@ -2,6 +2,7 @@ import platform import subprocess import sys +import traceback import unicodedata from typing import Optional @@ -97,3 +98,13 @@ def is_safe(chr: str) -> bool: else: sanitized_str += "�" return sanitized_str + + +def format_exception(e: Exception) -> str: + # The signature of traceback.format_exception has changed in python 3.10 + if sys.version_info < (3, 10): + output = traceback.format_exception(*sys.exc_info()) + else: + output = traceback.format_exception(e) + + return "".join(output) diff --git a/poetry.lock b/poetry.lock index f611b0255..02dbaca36 100644 --- a/poetry.lock +++ b/poetry.lock @@ -875,6 +875,25 @@ pytest = "*" dev = ["pre-commit", "tox"] doc = ["sphinx", "sphinx-rtd-theme"] +[[package]] +name = "pytest-subprocess" +version = "1.5.2" +description = "A plugin to fake subprocess for pytest" +optional = false +python-versions = ">=3.6" +files = [ + {file = "pytest_subprocess-1.5.2-py3-none-any.whl", hash = "sha256:23ac7732aa8bd45f1757265b1316eb72a7f55b41fb21e2ca22e149ba3629fa46"}, + {file = "pytest_subprocess-1.5.2.tar.gz", hash = "sha256:ad3ca8a35e798bf9c82d9f16d88700b30d98c5a28236117b86c5d6e581a8ed97"}, +] + +[package.dependencies] +pytest = ">=4.0.0" + +[package.extras] +dev = ["changelogd", "nox"] +docs = ["changelogd", "furo", "sphinx", "sphinx-autodoc-typehints", "sphinxcontrib-napoleon"] +test = ["Pygments (>=2.0)", "anyio", "coverage", "docutils (>=0.12)", "pytest (>=4.0)", "pytest-asyncio (>=0.15.1)", "pytest-rerunfailures", "pytest-timeout"] + [[package]] name = "pywin32" version = "306" @@ -1080,4 +1099,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = ">=3.9,<3.13" -content-hash = "9f1c256ac7a768845d519e58206bb3021be7fca94a55c29534cb7a157609e4e8" +content-hash = "4644fe1a1c39aea54bed5283e381ddf0042bd5f9f899d3547b5b61d9cf60daad" diff --git a/pyproject.toml b/pyproject.toml index d240ceb9c..c26361ed0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,7 @@ pytest-mock = "^3.10.0" pytest-qt = "^4.2.0" pytest-cov = "^5.0.0" strip-ansi = "*" +pytest-subprocess = "^1.5.2" [tool.poetry.group.qubes.dependencies] pymupdf = "^1.23.6" diff --git a/tests/gui/test_main_window.py b/tests/gui/test_main_window.py index 2fe29d979..7c47a3224 100644 --- a/tests/gui/test_main_window.py +++ b/tests/gui/test_main_window.py @@ -1,11 +1,13 @@ import os import pathlib +import platform import shutil import time from typing import List from pytest import MonkeyPatch, fixture from pytest_mock import MockerFixture +from pytest_subprocess import FakeProcess from pytestqt.qtbot import QtBot from dangerzone.document import Document @@ -13,12 +15,17 @@ from dangerzone.gui import main_window as main_window_module from dangerzone.gui import updater as updater_module from dangerzone.gui.logic import DangerzoneGui -from dangerzone.gui.main_window import ( # import Pyside related objects from here to avoid duplicating import logic. + +# import Pyside related objects from here to avoid duplicating import logic. +from dangerzone.gui.main_window import ( ContentWidget, + InstallContainerThread, QtCore, QtGui, + WaitingWidgetContainer, ) from dangerzone.gui.updater import UpdateReport, UpdaterThread +from dangerzone.isolation_provider.container import Container from .test_updater import assert_report_equal, default_updater_settings @@ -492,3 +499,87 @@ def test_drop_1_invalid_2_valid_documents( content_widget.doc_selection_wrapper.dropEvent( drag_1_invalid_and_2_valid_files_event ) + + +def test_installation_image_ls_error( + qtbot: QtBot, mocker: MockerFixture, fp: FakeProcess +): + # Setup + mock_app = mocker.MagicMock() + dummy = mocker.MagicMock(spec=Container) + dummy.get_runtime.return_value = "podman" + + # Make "podman image ls" return an error + fp.register_subprocess( + ["podman", "image", "ls"], returncode=-1, stderr="podman image ls logs" + ) + fp.pass_command(["xdg-mime", "query", "default", "application/pdf"]) + + dz = DangerzoneGui(mock_app, dummy) + widget = WaitingWidgetContainer(dz) + qtbot.addWidget(widget) + + # Assert that the error is displayed in the GUI + if platform.system() in ["Darwin", "Windows"]: + assert "Dangerzone requires Docker Desktop" in widget.label.text() + else: + assert "Podman is installed but cannot run properly" in widget.label.text() + + assert "podman image ls logs" in widget.traceback.toPlainText() + + +def test_installation_failure_exception( + qtbot: QtBot, mocker: MockerFixture, fp: FakeProcess +): + # Setup + mock_app = mocker.MagicMock() + dummy = mocker.MagicMock(spec=Container) + dummy.get_runtime.return_value = "podman" + dummy.install.side_effect = RuntimeError("Error during install") + + # Make "podman image ls" return an error + fp.register_subprocess(["podman", "image", "ls"], returncode=0) + fp.pass_command(["xdg-mime", "query", "default", "application/pdf"]) + + dz = DangerzoneGui(mock_app, dummy) + widget = WaitingWidgetContainer(dz) + qtbot.addWidget(widget) + + # Assert that the error is displayed in the GUI + assert "Installing the Dangerzone container image" in widget.label.text() + # Start the installation process + install_thread = InstallContainerThread(dz) + with qtbot.waitSignal(install_thread.finished, timeout=5000): + install_thread.start() + + assert "the following error occured" in widget.label.text() + assert "Traceback" in widget.traceback.toPlainText() + assert "Error during install" in widget.traceback.toPlainText() + + +def test_installation_failure_return_false( + qtbot: QtBot, mocker: MockerFixture, fp: FakeProcess +): + # Setup + mock_app = mocker.MagicMock() + dummy = mocker.MagicMock(spec=Container) + dummy.get_runtime.return_value = "podman" + dummy.install.return_value = False + + # Make "podman image ls" return an error + fp.register_subprocess(["podman", "image", "ls"], returncode=0) + fp.pass_command(["xdg-mime", "query", "default", "application/pdf"]) + + dz = DangerzoneGui(mock_app, dummy) + widget = WaitingWidgetContainer(dz) + qtbot.addWidget(widget) + + # Assert that the error is displayed in the GUI + assert "Installing the Dangerzone container image" in widget.label.text() + # Start the installation process + install_thread = InstallContainerThread(dz) + with qtbot.waitSignal(install_thread.finished, timeout=5000): + install_thread.start() + + assert "the following error occured" in widget.label.text() + assert "The image cannot be found" in widget.traceback.toPlainText() diff --git a/tests/isolation_provider/base.py b/tests/isolation_provider/base.py index abf488961..50199d7db 100644 --- a/tests/isolation_provider/base.py +++ b/tests/isolation_provider/base.py @@ -7,7 +7,6 @@ from dangerzone.conversion import errors from dangerzone.document import Document from dangerzone.isolation_provider import base -from dangerzone.isolation_provider.qubes import running_on_qubes TIMEOUT_STARTUP = 60 # Timeout in seconds until the conversion sandbox starts.