From 75ba097eca265bb4c2bed41370ec5bb39f14c276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 21 Mar 2025 14:22:24 +0100 Subject: [PATCH 01/13] Use `pathlib.Path` to return path locations --- dangerzone/container_utils.py | 4 +-- dangerzone/gui/__init__.py | 2 +- dangerzone/gui/logic.py | 4 +-- dangerzone/gui/main_window.py | 9 +++--- dangerzone/isolation_provider/container.py | 2 +- dangerzone/isolation_provider/qubes.py | 3 +- dangerzone/logic.py | 2 +- dangerzone/util.py | 35 +++++++++++----------- tests/isolation_provider/test_container.py | 4 +-- tests/test_util.py | 2 +- 10 files changed, 32 insertions(+), 35 deletions(-) diff --git a/dangerzone/container_utils.py b/dangerzone/container_utils.py index d651f0a..5794764 100644 --- a/dangerzone/container_utils.py +++ b/dangerzone/container_utils.py @@ -121,7 +121,7 @@ def delete_image_tag(tag: str) -> None: def get_expected_tag() -> str: """Get the tag of the Dangerzone image tarball from the image-id.txt file.""" - with open(get_resource_path("image-id.txt")) as f: + with get_resource_path("image-id.txt").open() as f: return f.read().strip() @@ -130,7 +130,7 @@ def load_image_tarball() -> None: tarball_path = get_resource_path("container.tar") try: res = subprocess.run( - [get_runtime(), "load", "-i", tarball_path], + [get_runtime(), "load", "-i", str(tarball_path)], startupinfo=get_subprocess_startupinfo(), capture_output=True, check=True, diff --git a/dangerzone/gui/__init__.py b/dangerzone/gui/__init__.py index 0f51759..5a4c26c 100644 --- a/dangerzone/gui/__init__.py +++ b/dangerzone/gui/__init__.py @@ -51,7 +51,7 @@ class Application(QtWidgets.QApplication): def __init__(self, *args: typing.Any, **kwargs: typing.Any) -> None: super(Application, self).__init__(*args, **kwargs) self.setQuitOnLastWindowClosed(False) - with open(get_resource_path("dangerzone.css"), "r") as f: + with get_resource_path("dangerzone.css").open("r") as f: style = f.read() self.setStyleSheet(style) diff --git a/dangerzone/gui/logic.py b/dangerzone/gui/logic.py index 9721043..b8403b1 100644 --- a/dangerzone/gui/logic.py +++ b/dangerzone/gui/logic.py @@ -63,7 +63,7 @@ class DangerzoneGui(DangerzoneCore): path = get_resource_path("dangerzone.ico") else: path = get_resource_path("icon.png") - return QtGui.QIcon(path) + return QtGui.QIcon(str(path)) def open_pdf_viewer(self, filename: str) -> None: if platform.system() == "Darwin": @@ -252,7 +252,7 @@ class Alert(Dialog): def create_layout(self) -> QtWidgets.QBoxLayout: logo = QtWidgets.QLabel() logo.setPixmap( - QtGui.QPixmap.fromImage(QtGui.QImage(get_resource_path("icon.png"))) + QtGui.QPixmap.fromImage(QtGui.QImage(str(get_resource_path("icon.png")))) ) label = QtWidgets.QLabel() diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index 0728961..9e4e23b 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -62,7 +62,7 @@ def load_svg_image(filename: str, width: int, height: int) -> QtGui.QPixmap: This answer is basically taken from: https://stackoverflow.com/a/25689790 """ path = get_resource_path(filename) - svg_renderer = QtSvg.QSvgRenderer(path) + svg_renderer = QtSvg.QSvgRenderer(str(path)) image = QtGui.QImage(width, height, QtGui.QImage.Format_ARGB32) # Set the ARGB to 0 to prevent rendering artifacts image.fill(0x00000000) @@ -130,9 +130,8 @@ class MainWindow(QtWidgets.QMainWindow): # Header logo = QtWidgets.QLabel() - logo.setPixmap( - QtGui.QPixmap.fromImage(QtGui.QImage(get_resource_path("icon.png"))) - ) + icon_path = str(get_resource_path("icon.png")) + logo.setPixmap(QtGui.QPixmap.fromImage(QtGui.QImage(icon_path))) header_label = QtWidgets.QLabel("Dangerzone") header_label.setFont(self.dangerzone.fixed_font) header_label.setStyleSheet("QLabel { font-weight: bold; font-size: 50px; }") @@ -1306,7 +1305,7 @@ class DocumentWidget(QtWidgets.QWidget): def load_status_image(self, filename: str) -> QtGui.QPixmap: path = get_resource_path(filename) - img = QtGui.QImage(path) + img = QtGui.QImage(str(path)) image = QtGui.QPixmap.fromImage(img) return image.scaled(QtCore.QSize(15, 15)) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index 1cd80cc..c133442 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -64,7 +64,7 @@ class Container(IsolationProvider): # # [1] https://github.com/freedomofpress/dangerzone/issues/846 # [2] https://github.com/containers/common/blob/d3283f8401eeeb21f3c59a425b5461f069e199a7/pkg/seccomp/seccomp.json - seccomp_json_path = get_resource_path("seccomp.gvisor.json") + seccomp_json_path = str(get_resource_path("seccomp.gvisor.json")) security_args += ["--security-opt", f"seccomp={seccomp_json_path}"] security_args += ["--cap-drop", "all"] diff --git a/dangerzone/isolation_provider/qubes.py b/dangerzone/isolation_provider/qubes.py index 4d2d5ee..d46e6cc 100644 --- a/dangerzone/isolation_provider/qubes.py +++ b/dangerzone/isolation_provider/qubes.py @@ -130,7 +130,6 @@ def is_qubes_native_conversion() -> bool: # This disambiguates if it is running a Qubes targetted build or not # (Qubes-specific builds don't ship the container image) - container_image_path = get_resource_path("container.tar") - return not os.path.exists(container_image_path) + return not get_resource_path("container.tar").exists() else: return False diff --git a/dangerzone/logic.py b/dangerzone/logic.py index eb588b9..1ec94d3 100644 --- a/dangerzone/logic.py +++ b/dangerzone/logic.py @@ -27,7 +27,7 @@ class DangerzoneCore(object): self.appdata_path = util.get_config_dir() # Languages supported by tesseract - with open(get_resource_path("ocr-languages.json"), "r") as f: + with get_resource_path("ocr-languages.json").open("r") as f: unsorted_ocr_languages = json.load(f) self.ocr_languages = dict(sorted(unsorted_ocr_languages.items())) diff --git a/dangerzone/util.py b/dangerzone/util.py index 1fa3025..6cae643 100644 --- a/dangerzone/util.py +++ b/dangerzone/util.py @@ -1,9 +1,9 @@ -import pathlib import platform import subprocess import sys import traceback import unicodedata +from pathlib import Path try: import platformdirs @@ -11,40 +11,39 @@ except ImportError: import appdirs as platformdirs -def get_config_dir() -> str: - return platformdirs.user_config_dir("dangerzone") +def get_config_dir() -> Path: + return Path(platformdirs.user_config_dir("dangerzone")) -def get_resource_path(filename: str) -> str: +def get_resource_path(filename: str) -> Path: if getattr(sys, "dangerzone_dev", False): # Look for resources directory relative to python file - project_root = pathlib.Path(__file__).parent.parent + project_root = Path(__file__).parent.parent prefix = project_root / "share" else: if platform.system() == "Darwin": - bin_path = pathlib.Path(sys.executable) + bin_path = Path(sys.executable) app_path = bin_path.parent.parent prefix = app_path / "Resources" / "share" elif platform.system() == "Linux": - prefix = pathlib.Path(sys.prefix) / "share" / "dangerzone" + prefix = Path(sys.prefix) / "share" / "dangerzone" elif platform.system() == "Windows": - exe_path = pathlib.Path(sys.executable) + exe_path = Path(sys.executable) dz_install_path = exe_path.parent prefix = dz_install_path / "share" else: raise NotImplementedError(f"Unsupported system {platform.system()}") - resource_path = prefix / filename - return str(resource_path) + return prefix / filename -def get_tessdata_dir() -> pathlib.Path: +def get_tessdata_dir() -> Path: if getattr(sys, "dangerzone_dev", False) or platform.system() in ( "Windows", "Darwin", ): # Always use the tessdata path from the Dangerzone ./share directory, for # development builds, or in Windows/macOS platforms. - return pathlib.Path(get_resource_path("tessdata")) + return get_resource_path("tessdata") # In case of Linux systems, grab the Tesseract data from any of the following # locations. We have found some of the locations through trial and error, whereas @@ -55,11 +54,11 @@ def get_tessdata_dir() -> pathlib.Path: # # [1] https://tesseract-ocr.github.io/tessdoc/Installation.html tessdata_dirs = [ - pathlib.Path("/usr/share/tessdata/"), # on some Debian - pathlib.Path("/usr/share/tesseract/tessdata/"), # on Fedora - pathlib.Path("/usr/share/tesseract-ocr/tessdata/"), # ? (documented) - pathlib.Path("/usr/share/tesseract-ocr/4.00/tessdata/"), # on Debian Bullseye - pathlib.Path("/usr/share/tesseract-ocr/5/tessdata/"), # on Debian Trixie + Path("/usr/share/tessdata/"), # on some Debian + Path("/usr/share/tesseract/tessdata/"), # on Fedora + Path("/usr/share/tesseract-ocr/tessdata/"), # ? (documented) + Path("/usr/share/tesseract-ocr/4.00/tessdata/"), # on Debian Bullseye + Path("/usr/share/tesseract-ocr/5/tessdata/"), # on Debian Trixie ] for dir in tessdata_dirs: @@ -71,7 +70,7 @@ def get_tessdata_dir() -> pathlib.Path: def get_version() -> str: try: - with open(get_resource_path("version.txt")) as f: + with get_resource_path("version.txt").open() as f: version = f.read().strip() except FileNotFoundError: # In dev mode, in Windows, get_resource_path doesn't work properly for the container, but luckily diff --git a/tests/isolation_provider/test_container.py b/tests/isolation_provider/test_container.py index c8c9655..7f39664 100644 --- a/tests/isolation_provider/test_container.py +++ b/tests/isolation_provider/test_container.py @@ -74,7 +74,7 @@ class TestContainer(IsolationProviderTest): container_utils.get_runtime(), "load", "-i", - get_resource_path("container.tar"), + get_resource_path("container.tar").absolute(), ], returncode=-1, ) @@ -113,7 +113,7 @@ class TestContainer(IsolationProviderTest): container_utils.get_runtime(), "load", "-i", - get_resource_path("container.tar"), + get_resource_path("container.tar").absolute(), ], ) with pytest.raises(errors.ImageNotPresentException): diff --git a/tests/test_util.py b/tests/test_util.py index 13d36ee..a8726bd 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -11,7 +11,7 @@ VERSION_FILE_NAME = "version.txt" def test_get_resource_path() -> None: share_dir = Path("share").resolve() - resource_path = Path(util.get_resource_path(VERSION_FILE_NAME)).parent + resource_path = util.get_resource_path(VERSION_FILE_NAME).parent assert share_dir.samefile(resource_path), ( f"{share_dir} is not the same file as {resource_path}" ) From 162e528f9768e6caf6c8178a3380d411f82c0166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 21 Mar 2025 15:41:58 +0100 Subject: [PATCH 02/13] Decouple the `Settings` class from `DangerzoneCore` No real reason to pass the whole object where what we really need is just the location of the configuration folder. --- dangerzone/logic.py | 5 +--- dangerzone/settings.py | 16 ++++-------- tests/test_settings.py | 58 +++++++++++++++--------------------------- 3 files changed, 27 insertions(+), 52 deletions(-) diff --git a/dangerzone/logic.py b/dangerzone/logic.py index 1ec94d3..74884af 100644 --- a/dangerzone/logic.py +++ b/dangerzone/logic.py @@ -23,16 +23,13 @@ class DangerzoneCore(object): # Initialize terminal colors colorama.init(autoreset=True) - # App data folder - self.appdata_path = util.get_config_dir() - # Languages supported by tesseract with get_resource_path("ocr-languages.json").open("r") as f: unsorted_ocr_languages = json.load(f) self.ocr_languages = dict(sorted(unsorted_ocr_languages.items())) # Load settings - self.settings = Settings(self) + self.settings = Settings() self.documents: List[Document] = [] self.isolation_provider = isolation_provider diff --git a/dangerzone/settings.py b/dangerzone/settings.py index 32026d6..4c8b939 100644 --- a/dangerzone/settings.py +++ b/dangerzone/settings.py @@ -6,24 +6,18 @@ from typing import TYPE_CHECKING, Any, Dict from packaging import version from .document import SAFE_EXTENSION -from .util import get_version +from .util import get_config_dir, get_version log = logging.getLogger(__name__) -if TYPE_CHECKING: - from .logic import DangerzoneCore - SETTINGS_FILENAME: str = "settings.json" class Settings: settings: Dict[str, Any] - def __init__(self, dangerzone: "DangerzoneCore") -> None: - self.dangerzone = dangerzone - self.settings_filename = os.path.join( - self.dangerzone.appdata_path, SETTINGS_FILENAME - ) + def __init__(self) -> None: + self.settings_filename = get_config_dir() / SETTINGS_FILENAME self.default_settings: Dict[str, Any] = self.generate_default_settings() self.load() @@ -91,6 +85,6 @@ class Settings: self.save() def save(self) -> None: - os.makedirs(self.dangerzone.appdata_path, exist_ok=True) - with open(self.settings_filename, "w") as settings_file: + self.settings_filename.parent.mkdir(parents=True, exist_ok=True) + with self.settings_filename.open("w") as settings_file: json.dump(self.settings, settings_file, indent=4) diff --git a/tests/test_settings.py b/tests/test_settings.py index 13deadd..87e20ec 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -1,5 +1,4 @@ import json -import os from pathlib import Path from unittest.mock import PropertyMock @@ -22,40 +21,31 @@ def default_settings_0_4_1() -> dict: } -@pytest.fixture -def settings(tmp_path: Path, mocker: MockerFixture) -> Settings: - dz_core = mocker.MagicMock() - type(dz_core).appdata_path = PropertyMock(return_value=tmp_path) - return Settings(dz_core) - - -def save_settings(tmp_path: Path, settings: dict) -> None: - """Mimic the way Settings save a dictionary to a settings.json file.""" - settings_filename = tmp_path / "settings.json" - with open(settings_filename, "w") as settings_file: - json.dump(settings, settings_file, indent=4) - - -def test_no_settings_file_creates_new_one(settings: Settings) -> None: +def test_no_settings_file_creates_new_one( + tmp_path: Path, + mocker: MockerFixture, +) -> None: """Default settings file is created on first run""" - assert os.path.isfile(settings.settings_filename) - new_settings_dict = json.load(open(settings.settings_filename)) - assert sorted(new_settings_dict.items()) == sorted( - settings.generate_default_settings().items() - ) + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + settings = Settings() + + assert settings.settings_filename.is_file() + with settings.settings_filename.open() as settings_file: + new_settings_dict = json.load(settings_file) + assert sorted(new_settings_dict.items()) == sorted( + settings.generate_default_settings().items() + ) def test_corrupt_settings(tmp_path: Path, mocker: MockerFixture) -> None: # Set some broken settings file corrupt_settings_dict = "{:}" - with open(tmp_path / SETTINGS_FILENAME, "w") as settings_file: + with (tmp_path / SETTINGS_FILENAME).open("w") as settings_file: settings_file.write(corrupt_settings_dict) - # Initialize settings - dz_core = mocker.MagicMock() - type(dz_core).appdata_path = PropertyMock(return_value=tmp_path) - settings = Settings(dz_core) - assert os.path.isfile(settings.settings_filename) + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + settings = Settings() + assert settings.settings_filename.is_file() # Check if settings file was reset to the default new_settings_dict = json.load(open(settings.settings_filename)) @@ -66,10 +56,7 @@ def test_corrupt_settings(tmp_path: Path, mocker: MockerFixture) -> None: def test_new_default_setting(tmp_path: Path, mocker: MockerFixture) -> None: - # Initialize settings - dz_core = mocker.MagicMock() - type(dz_core).appdata_path = PropertyMock(return_value=tmp_path) - settings = Settings(dz_core) + settings = Settings() settings.save() # Ensure new default setting is imported into settings @@ -78,15 +65,12 @@ def test_new_default_setting(tmp_path: Path, mocker: MockerFixture) -> None: return_value={"mock_setting": 1}, ) - settings2 = Settings(dz_core) + settings2 = Settings() assert settings2.get("mock_setting") == 1 def test_new_settings_added(tmp_path: Path, mocker: MockerFixture) -> None: - # Initialize settings - dz_core = mocker.MagicMock() - type(dz_core).appdata_path = PropertyMock(return_value=tmp_path) - settings = Settings(dz_core) + settings = Settings() # Add new setting settings.set("new_setting_autosaved", 20, autosave=True) @@ -95,7 +79,7 @@ def test_new_settings_added(tmp_path: Path, mocker: MockerFixture) -> None: ) # XXX has to be afterwards; otherwise this will be saved # Simulate new app startup (settings recreation) - settings2 = Settings(dz_core) + settings2 = Settings() # Check if new setting persisted assert 20 == settings2.get("new_setting_autosaved") From 55bd68ce1edd838e61a5f3abc7d0db9253f04617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 24 Mar 2025 16:20:04 +0100 Subject: [PATCH 03/13] Mock the settings rather than monkeypatching external modules --- tests/gui/conftest.py | 27 +++++++++------------------ tests/gui/test_updater.py | 18 +++++------------- tests/test_settings.py | 7 +++++++ 3 files changed, 21 insertions(+), 31 deletions(-) diff --git a/tests/gui/conftest.py b/tests/gui/conftest.py index cbf49de..28df274 100644 --- a/tests/gui/conftest.py +++ b/tests/gui/conftest.py @@ -21,34 +21,25 @@ def get_qt_app() -> Application: def generate_isolated_updater( tmp_path: Path, - monkeypatch: MonkeyPatch, - app_mocker: Optional[MockerFixture] = None, + mocker: MockerFixture, + mock_app: bool = False, ) -> UpdaterThread: """Generate an Updater class with its own settings.""" - if app_mocker: - app = app_mocker.MagicMock() - else: - app = get_qt_app() + app = mocker.MagicMock() if mock_app else get_qt_app() dummy = Dummy() - # XXX: We can monkey-patch global state without wrapping it in a context manager, or - # worrying that it will leak between tests, for two reasons: - # - # 1. Parallel tests in PyTest take place in different processes. - # 2. The monkeypatch fixture tears down the monkey-patch after each test ends. - monkeypatch.setattr(util, "get_config_dir", lambda: tmp_path) + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + dangerzone = DangerzoneGui(app, isolation_provider=dummy) updater = UpdaterThread(dangerzone) return updater @pytest.fixture -def updater( - tmp_path: Path, monkeypatch: MonkeyPatch, mocker: MockerFixture -) -> UpdaterThread: - return generate_isolated_updater(tmp_path, monkeypatch, mocker) +def updater(tmp_path: Path, mocker: MockerFixture) -> UpdaterThread: + return generate_isolated_updater(tmp_path, mocker, mock_app=True) @pytest.fixture -def qt_updater(tmp_path: Path, monkeypatch: MonkeyPatch) -> UpdaterThread: - return generate_isolated_updater(tmp_path, monkeypatch) +def qt_updater(tmp_path: Path, mocker: MockerFixture) -> UpdaterThread: + return generate_isolated_updater(tmp_path, mocker, mock_app=False) diff --git a/tests/gui/test_updater.py b/tests/gui/test_updater.py index 430a621..9bf544a 100644 --- a/tests/gui/test_updater.py +++ b/tests/gui/test_updater.py @@ -48,9 +48,7 @@ def test_default_updater_settings(updater: UpdaterThread) -> None: ) -def test_pre_0_4_2_settings( - tmp_path: Path, monkeypatch: MonkeyPatch, mocker: MockerFixture -) -> None: +def test_pre_0_4_2_settings(tmp_path: Path, mocker: MockerFixture) -> None: """Check settings of installations prior to 0.4.2. Check that installations that have been upgraded from a version < 0.4.2 to >= 0.4.2 @@ -58,7 +56,7 @@ def test_pre_0_4_2_settings( in their settings.json file. """ save_settings(tmp_path, default_settings_0_4_1()) - updater = generate_isolated_updater(tmp_path, monkeypatch, mocker) + updater = generate_isolated_updater(tmp_path, mocker, mock_app=True) assert ( updater.dangerzone.settings.get_updater_settings() == default_updater_settings() ) @@ -83,12 +81,10 @@ def test_post_0_4_2_settings( # version is 0.4.3. expected_settings = default_updater_settings() expected_settings["updater_latest_version"] = "0.4.3" - monkeypatch.setattr( - settings, "get_version", lambda: expected_settings["updater_latest_version"] - ) + monkeypatch.setattr(settings, "get_version", lambda: "0.4.3") # Ensure that the Settings class will correct the latest version field to 0.4.3. - updater = generate_isolated_updater(tmp_path, monkeypatch, mocker) + updater = generate_isolated_updater(tmp_path, mocker, mock_app=True) assert updater.dangerzone.settings.get_updater_settings() == expected_settings # Simulate an updater check that found a newer Dangerzone version (e.g., 0.4.4). @@ -118,9 +114,7 @@ def test_linux_no_check(updater: UpdaterThread, monkeypatch: MonkeyPatch) -> Non assert updater.dangerzone.settings.get_updater_settings() == expected_settings -def test_user_prompts( - updater: UpdaterThread, monkeypatch: MonkeyPatch, mocker: MockerFixture -) -> None: +def test_user_prompts(updater: UpdaterThread, mocker: MockerFixture) -> None: """Test prompting users to ask them if they want to enable update checks.""" # First run # @@ -370,8 +364,6 @@ def test_update_errors( def test_update_check_prompt( qtbot: QtBot, qt_updater: UpdaterThread, - monkeypatch: MonkeyPatch, - mocker: MockerFixture, ) -> None: """Test that the prompt to enable update checks works properly.""" # Force Dangerzone to check immediately for updates diff --git a/tests/test_settings.py b/tests/test_settings.py index 87e20ec..58906b7 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -21,6 +21,13 @@ def default_settings_0_4_1() -> dict: } +def save_settings(tmp_path: Path, settings: dict) -> None: + """Mimic the way Settings save a dictionary to a settings.json file.""" + settings_filename = tmp_path / "settings.json" + with open(settings_filename, "w") as settings_file: + json.dump(settings, settings_file, indent=4) + + def test_no_settings_file_creates_new_one( tmp_path: Path, mocker: MockerFixture, From c142b72ab27e503310163dcba41f9482baa161f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 24 Mar 2025 16:39:39 +0100 Subject: [PATCH 04/13] Allow to read the container runtime from the settings Add a few tests for this along the way, and update the end-user messages about Docker/Podman to account for this change. --- dangerzone/container_utils.py | 38 ++++++++++++---------- dangerzone/gui/main_window.py | 16 +++++++-- dangerzone/settings.py | 3 ++ tests/isolation_provider/test_container.py | 2 +- tests/test_container_utils.py | 28 ++++++++++++++++ 5 files changed, 66 insertions(+), 21 deletions(-) create mode 100644 tests/test_container_utils.py diff --git a/dangerzone/container_utils.py b/dangerzone/container_utils.py index 5794764..bbdb75a 100644 --- a/dangerzone/container_utils.py +++ b/dangerzone/container_utils.py @@ -5,6 +5,7 @@ import subprocess from typing import List, Tuple from . import errors +from .settings import Settings from .util import get_resource_path, get_subprocess_startupinfo CONTAINER_NAME = "dangerzone.rocks/dangerzone" @@ -13,14 +14,22 @@ log = logging.getLogger(__name__) def get_runtime_name() -> str: - if platform.system() == "Linux": - runtime_name = "podman" - else: - # Windows, Darwin, and unknown use docker for now, dangerzone-vm eventually - runtime_name = "docker" + settings = Settings() + try: + runtime_name = settings.get("container_runtime") + except KeyError: + return "podman" if platform.system() == "Linux" else "docker" return runtime_name +def get_runtime() -> str: + container_tech = get_runtime_name() + runtime = shutil.which(container_tech) + if runtime is None: + raise errors.NoContainerTechException(container_tech) + return runtime + + def get_runtime_version() -> Tuple[int, int]: """Get the major/minor parts of the Docker/Podman version. @@ -31,13 +40,14 @@ def get_runtime_version() -> Tuple[int, int]: semver parser is an overkill. """ # Get the Docker/Podman version, using a Go template. - runtime = get_runtime_name() - if runtime == "podman": + runtime_name = get_runtime_name() + + if runtime_name == "podman": query = "{{.Client.Version}}" else: query = "{{.Server.Version}}" - cmd = [runtime, "version", "-f", query] + cmd = [get_runtime(), "version", "-f", query] try: version = subprocess.run( cmd, @@ -46,7 +56,7 @@ def get_runtime_version() -> Tuple[int, int]: check=True, ).stdout.decode() except Exception as e: - msg = f"Could not get the version of the {runtime.capitalize()} tool: {e}" + msg = f"Could not get the version of the {runtime.name.capitalize()} tool: {e}" raise RuntimeError(msg) from e # Parse this version and return the major/minor parts, since we don't need the @@ -56,20 +66,12 @@ def get_runtime_version() -> Tuple[int, int]: return (int(major), int(minor)) except Exception as e: msg = ( - f"Could not parse the version of the {runtime.capitalize()} tool" + f"Could not parse the version of the {runtime_name.capitalize()} tool" f" (found: '{version}') due to the following error: {e}" ) raise RuntimeError(msg) -def get_runtime() -> str: - container_tech = get_runtime_name() - runtime = shutil.which(container_tech) - if runtime is None: - raise errors.NoContainerTechException(container_tech) - return runtime - - def list_image_tags() -> List[str]: """Get the tags of all loaded Dangerzone images. diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py index 9e4e23b..d162fcf 100644 --- a/dangerzone/gui/main_window.py +++ b/dangerzone/gui/main_window.py @@ -574,8 +574,15 @@ class WaitingWidgetContainer(WaitingWidget): self.finished.emit() def state_change(self, state: str, error: Optional[str] = None) -> None: + custom_runtime = self.dangerzone.settings.custom_runtime_specified() + if state == "not_installed": - if platform.system() == "Linux": + if custom_runtime: + self.show_error( + "We could not find the container runtime defined in your settings

" + "Please check your settings, install it if needed, and retry." + ) + elif platform.system() == "Linux": self.show_error( "Dangerzone requires Podman

" "Install it and retry." @@ -588,7 +595,12 @@ class WaitingWidgetContainer(WaitingWidget): ) elif state == "not_running": - if platform.system() == "Linux": + if custom_runtime: + self.show_error( + "We were unable to start the container runtime defined in your settings

" + "Please check your settings, install it if needed, and retry." + ) + elif platform.system() == "Linux": # "not_running" here means that the `podman image ls` command failed. message = ( "Dangerzone requires Podman

" diff --git a/dangerzone/settings.py b/dangerzone/settings.py index 4c8b939..a10ad6b 100644 --- a/dangerzone/settings.py +++ b/dangerzone/settings.py @@ -39,6 +39,9 @@ class Settings: "updater_errors": 0, } + def custom_runtime_specified(self) -> bool: + return "container_runtime" in self.settings + def get(self, key: str) -> Any: return self.settings[key] diff --git a/tests/isolation_provider/test_container.py b/tests/isolation_provider/test_container.py index 7f39664..797986e 100644 --- a/tests/isolation_provider/test_container.py +++ b/tests/isolation_provider/test_container.py @@ -87,7 +87,7 @@ class TestContainer(IsolationProviderTest): ) -> None: """When an image keep being not installed, it should return False""" fp.register_subprocess( - ["podman", "version", "-f", "{{.Client.Version}}"], + [container_utils.get_runtime(), "version", "-f", "{{.Client.Version}}"], stdout="4.0.0", ) diff --git a/tests/test_container_utils.py b/tests/test_container_utils.py new file mode 100644 index 0000000..e921507 --- /dev/null +++ b/tests/test_container_utils.py @@ -0,0 +1,28 @@ +from pathlib import Path + +from pytest_mock import MockerFixture + +from dangerzone.container_utils import get_runtime_name +from dangerzone.settings import Settings + + +def test_get_runtime_name_from_settings(mocker: MockerFixture, tmp_path: Path) -> None: + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + + settings = Settings() + settings.set("container_runtime", "new-kid-on-the-block", autosave=True) + + assert get_runtime_name() == "new-kid-on-the-block" + + +def test_get_runtime_name_linux(mocker: MockerFixture) -> None: + mocker.patch("platform.system", return_value="Linux") + assert get_runtime_name() == "podman" + + +def test_get_runtime_name_non_linux(mocker: MockerFixture) -> None: + mocker.patch("platform.system", return_value="Windows") + assert get_runtime_name() == "docker" + + mocker.patch("platform.system", return_value="Something else") + assert get_runtime_name() == "docker" From c250f495afa154476cfed3a486606fcfce379374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 24 Mar 2025 17:52:03 +0100 Subject: [PATCH 05/13] Only check Docker version if the container runtime is set to docker --- dangerzone/isolation_provider/container.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index c133442..cbb930a 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -143,7 +143,10 @@ class Container(IsolationProvider): def check_docker_desktop_version(self) -> Tuple[bool, str]: # On windows and darwin, check that the minimum version is met version = "" - if platform.system() != "Linux": + runtime_is_docker = container_utils.get_runtime_name() == "docker" + platform_is_not_linux = platform.system() != "Linux" + + if runtime_is_docker and platform_is_not_linux: with subprocess.Popen( ["docker", "version", "--format", "{{.Server.Platform.Name}}"], stdout=subprocess.PIPE, From e34fa5cbb2486d22aa3888841c17de8b70c81393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 24 Mar 2025 18:03:32 +0100 Subject: [PATCH 06/13] Allow to define a container_runtime_path --- dangerzone/container_utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dangerzone/container_utils.py b/dangerzone/container_utils.py index bbdb75a..13641a2 100644 --- a/dangerzone/container_utils.py +++ b/dangerzone/container_utils.py @@ -1,4 +1,5 @@ import logging +import os import platform import shutil import subprocess @@ -26,6 +27,11 @@ def get_runtime() -> str: container_tech = get_runtime_name() runtime = shutil.which(container_tech) if runtime is None: + # Fallback to the container runtime path from the settings + settings = Settings() + runtime_path = settings.get("container_runtime_path") + if os.path.exists(runtime_path): + return runtime_path raise errors.NoContainerTechException(container_tech) return runtime From 0f45d84fe95197138c1cd33b9b12ab0b2439b98c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Tue, 25 Mar 2025 12:20:10 +0100 Subject: [PATCH 07/13] Use a `Runtime` class to get information about container runtimes This is useful to avoid parsing too many times the settings. --- dangerzone/container_utils.py | 64 +++++++++++----------- dangerzone/isolation_provider/container.py | 33 ++++++----- tests/isolation_provider/test_container.py | 38 ++++++++----- tests/test_container_utils.py | 36 +++++++++--- 4 files changed, 103 insertions(+), 68 deletions(-) diff --git a/dangerzone/container_utils.py b/dangerzone/container_utils.py index 13641a2..c645358 100644 --- a/dangerzone/container_utils.py +++ b/dangerzone/container_utils.py @@ -3,7 +3,8 @@ import os import platform import shutil import subprocess -from typing import List, Tuple +from pathlib import Path +from typing import List, Optional, Tuple from . import errors from .settings import Settings @@ -14,29 +15,26 @@ CONTAINER_NAME = "dangerzone.rocks/dangerzone" log = logging.getLogger(__name__) -def get_runtime_name() -> str: - settings = Settings() - try: - runtime_name = settings.get("container_runtime") - except KeyError: - return "podman" if platform.system() == "Linux" else "docker" - return runtime_name - - -def get_runtime() -> str: - container_tech = get_runtime_name() - runtime = shutil.which(container_tech) - if runtime is None: - # Fallback to the container runtime path from the settings +class Runtime(object): + def __init__(self) -> None: settings = Settings() - runtime_path = settings.get("container_runtime_path") - if os.path.exists(runtime_path): - return runtime_path - raise errors.NoContainerTechException(container_tech) - return runtime + + if settings.custom_runtime_specified(): + self.path = Path(settings.get("container_runtime")) + self.name = self.path.stem + else: + self.name = self.get_default_runtime_name() + binary_path = shutil.which(self.name) + if binary_path is None or not os.path.exists(binary_path): + raise errors.NoContainerTechException(self.name) + self.path = Path(binary_path) + + @staticmethod + def get_default_runtime_name() -> str: + return "podman" if platform.system() == "Linux" else "docker" -def get_runtime_version() -> Tuple[int, int]: +def get_runtime_version(runtime: Optional[Runtime] = None) -> Tuple[int, int]: """Get the major/minor parts of the Docker/Podman version. Some of the operations we perform in this module rely on some Podman features @@ -45,15 +43,15 @@ def get_runtime_version() -> Tuple[int, int]: just knowing the major and minor version, since writing/installing a full-blown semver parser is an overkill. """ - # Get the Docker/Podman version, using a Go template. - runtime_name = get_runtime_name() + runtime = runtime or Runtime() - if runtime_name == "podman": + # Get the Docker/Podman version, using a Go template. + if runtime.name == "podman": query = "{{.Client.Version}}" else: query = "{{.Server.Version}}" - cmd = [get_runtime(), "version", "-f", query] + cmd = [str(runtime.path), "version", "-f", query] try: version = subprocess.run( cmd, @@ -72,7 +70,7 @@ def get_runtime_version() -> Tuple[int, int]: return (int(major), int(minor)) except Exception as e: msg = ( - f"Could not parse the version of the {runtime_name.capitalize()} tool" + f"Could not parse the version of the {runtime.name.capitalize()} tool" f" (found: '{version}') due to the following error: {e}" ) raise RuntimeError(msg) @@ -85,10 +83,11 @@ def list_image_tags() -> List[str]: images. This can be useful when we want to find which are the local image tags, and which image ID does the "latest" tag point to. """ + runtime = Runtime() return ( subprocess.check_output( [ - get_runtime(), + str(runtime.path), "image", "list", "--format", @@ -105,19 +104,21 @@ def list_image_tags() -> List[str]: def add_image_tag(image_id: str, new_tag: str) -> None: """Add a tag to the Dangerzone image.""" + runtime = Runtime() log.debug(f"Adding tag '{new_tag}' to image '{image_id}'") subprocess.check_output( - [get_runtime(), "tag", image_id, new_tag], + [str(runtime.path), "tag", image_id, new_tag], startupinfo=get_subprocess_startupinfo(), ) def delete_image_tag(tag: str) -> None: """Delete a Dangerzone image tag.""" + runtime = Runtime() log.warning(f"Deleting old container image: {tag}") try: subprocess.check_output( - [get_runtime(), "rmi", "--force", tag], + [str(runtime.name), "rmi", "--force", tag], startupinfo=get_subprocess_startupinfo(), ) except Exception as e: @@ -134,11 +135,12 @@ def get_expected_tag() -> str: def load_image_tarball() -> None: + runtime = Runtime() log.info("Installing Dangerzone container image...") tarball_path = get_resource_path("container.tar") try: res = subprocess.run( - [get_runtime(), "load", "-i", str(tarball_path)], + [str(runtime.path), "load", "-i", str(tarball_path)], startupinfo=get_subprocess_startupinfo(), capture_output=True, check=True, @@ -163,7 +165,7 @@ def load_image_tarball() -> None: # `share/image-id.txt` and delete the incorrect tag. # # [1] https://github.com/containers/podman/issues/16490 - if get_runtime_name() == "podman" and get_runtime_version() == (3, 4): + if runtime.name == "podman" and get_runtime_version(runtime) == (3, 4): expected_tag = get_expected_tag() bad_tag = f"localhost/{expected_tag}:latest" good_tag = f"{CONTAINER_NAME}:{expected_tag}" diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index cbb930a..ed810f8 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -6,6 +6,7 @@ import subprocess from typing import List, Tuple from .. import container_utils, errors +from ..container_utils import Runtime from ..document import Document from ..util import get_resource_path, get_subprocess_startupinfo from .base import IsolationProvider, terminate_process_group @@ -50,7 +51,8 @@ class Container(IsolationProvider): * Do not map the host user to the container, with `--userns nomap` (available from Podman 4.1 onwards) """ - if container_utils.get_runtime_name() == "podman": + runtime = Runtime() + if runtime.name == "podman": security_args = ["--log-driver", "none"] security_args += ["--security-opt", "no-new-privileges"] if container_utils.get_runtime_version() >= (4, 1): @@ -123,12 +125,11 @@ class Container(IsolationProvider): @staticmethod def is_available() -> bool: - container_runtime = container_utils.get_runtime() - runtime_name = container_utils.get_runtime_name() + runtime = Runtime() # Can we run `docker/podman image ls` without an error with subprocess.Popen( - [container_runtime, "image", "ls"], + [str(runtime.path), "image", "ls"], stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, startupinfo=get_subprocess_startupinfo(), @@ -136,14 +137,15 @@ class Container(IsolationProvider): _, stderr = p.communicate() if p.returncode != 0: raise errors.NotAvailableContainerTechException( - runtime_name, stderr.decode() + runtime.name, stderr.decode() ) return True def check_docker_desktop_version(self) -> Tuple[bool, str]: # On windows and darwin, check that the minimum version is met version = "" - runtime_is_docker = container_utils.get_runtime_name() == "docker" + runtime = Runtime() + runtime_is_docker = runtime.name == "docker" platform_is_not_linux = platform.system() != "Linux" if runtime_is_docker and platform_is_not_linux: @@ -196,7 +198,7 @@ class Container(IsolationProvider): command: List[str], name: str, ) -> subprocess.Popen: - container_runtime = container_utils.get_runtime() + runtime = Runtime() security_args = self.get_runtime_security_args() debug_args = [] if self.debug: @@ -218,7 +220,7 @@ class Container(IsolationProvider): + image_name + command ) - return self.exec([container_runtime] + args) + return self.exec([str(runtime.path)] + args) def kill_container(self, name: str) -> None: """Terminate a spawned container. @@ -230,8 +232,8 @@ class Container(IsolationProvider): connected to the Docker daemon, and killing it will just close the associated standard streams. """ - container_runtime = container_utils.get_runtime() - cmd = [container_runtime, "kill", name] + runtime = Runtime() + cmd = [str(runtime.path), "kill", name] try: # We do not check the exit code of the process here, since the container may # have stopped right before invoking this command. In that case, the @@ -287,10 +289,10 @@ class Container(IsolationProvider): # after a podman kill / docker kill invocation, this will likely be the case, # else the container runtime (Docker/Podman) has experienced a problem, and we # should report it. - container_runtime = container_utils.get_runtime() + runtime = Runtime() name = self.doc_to_pixels_container_name(document) all_containers = subprocess.run( - [container_runtime, "ps", "-a"], + [str(runtime.path), "ps", "-a"], capture_output=True, startupinfo=get_subprocess_startupinfo(), ) @@ -301,19 +303,20 @@ class Container(IsolationProvider): # FIXME hardcoded 1 until length conversions are better handled # https://github.com/freedomofpress/dangerzone/issues/257 return 1 + runtime = Runtime() # type: ignore [unreachable] - n_cpu = 1 # type: ignore [unreachable] + n_cpu = 1 if platform.system() == "Linux": # if on linux containers run natively cpu_count = os.cpu_count() if cpu_count is not None: n_cpu = cpu_count - elif container_utils.get_runtime_name() == "docker": + elif runtime.name == "docker": # For Windows and MacOS containers run in VM # So we obtain the CPU count for the VM n_cpu_str = subprocess.check_output( - [container_utils.get_runtime(), "info", "--format", "{{.NCPU}}"], + [str(runtime.path), "info", "--format", "{{.NCPU}}"], text=True, startupinfo=get_subprocess_startupinfo(), ) diff --git a/tests/isolation_provider/test_container.py b/tests/isolation_provider/test_container.py index 797986e..8a5f170 100644 --- a/tests/isolation_provider/test_container.py +++ b/tests/isolation_provider/test_container.py @@ -5,7 +5,8 @@ import pytest from pytest_mock import MockerFixture from pytest_subprocess import FakeProcess -from dangerzone import container_utils, errors +from dangerzone import errors +from dangerzone.container_utils import Runtime from dangerzone.isolation_provider.container import Container from dangerzone.isolation_provider.qubes import is_qubes_native_conversion from dangerzone.util import get_resource_path @@ -24,42 +25,51 @@ def provider() -> Container: return Container() +@pytest.fixture +def runtime_path() -> str: + return str(Runtime().path) + + class TestContainer(IsolationProviderTest): - def test_is_available_raises(self, provider: Container, fp: FakeProcess) -> None: + def test_is_available_raises( + self, provider: Container, fp: FakeProcess, runtime_path: str + ) -> None: """ NotAvailableContainerTechException should be raised when the "podman image ls" command fails. """ fp.register_subprocess( - [container_utils.get_runtime(), "image", "ls"], + [runtime_path, "image", "ls"], returncode=-1, stderr="podman image ls logs", ) with pytest.raises(errors.NotAvailableContainerTechException): provider.is_available() - def test_is_available_works(self, provider: Container, fp: FakeProcess) -> None: + def test_is_available_works( + self, provider: Container, fp: FakeProcess, runtime_path: str + ) -> None: """ No exception should be raised when the "podman image ls" can return properly. """ fp.register_subprocess( - [container_utils.get_runtime(), "image", "ls"], + [runtime_path, "image", "ls"], ) provider.is_available() def test_install_raise_if_image_cant_be_installed( - self, provider: Container, fp: FakeProcess + self, provider: Container, fp: FakeProcess, runtime_path: str ) -> None: """When an image installation fails, an exception should be raised""" fp.register_subprocess( - [container_utils.get_runtime(), "image", "ls"], + [runtime_path, "image", "ls"], ) # First check should return nothing. fp.register_subprocess( [ - container_utils.get_runtime(), + runtime_path, "image", "list", "--format", @@ -71,7 +81,7 @@ class TestContainer(IsolationProviderTest): fp.register_subprocess( [ - container_utils.get_runtime(), + runtime_path, "load", "-i", get_resource_path("container.tar").absolute(), @@ -83,22 +93,22 @@ class TestContainer(IsolationProviderTest): provider.install() def test_install_raises_if_still_not_installed( - self, provider: Container, fp: FakeProcess + self, provider: Container, fp: FakeProcess, runtime_path: str ) -> None: """When an image keep being not installed, it should return False""" fp.register_subprocess( - [container_utils.get_runtime(), "version", "-f", "{{.Client.Version}}"], + [runtime_path, "version", "-f", "{{.Client.Version}}"], stdout="4.0.0", ) fp.register_subprocess( - [container_utils.get_runtime(), "image", "ls"], + [runtime_path, "image", "ls"], ) # First check should return nothing. fp.register_subprocess( [ - container_utils.get_runtime(), + runtime_path, "image", "list", "--format", @@ -110,7 +120,7 @@ class TestContainer(IsolationProviderTest): fp.register_subprocess( [ - container_utils.get_runtime(), + runtime_path, "load", "-i", get_resource_path("container.tar").absolute(), diff --git a/tests/test_container_utils.py b/tests/test_container_utils.py index e921507..570bae1 100644 --- a/tests/test_container_utils.py +++ b/tests/test_container_utils.py @@ -2,7 +2,7 @@ from pathlib import Path from pytest_mock import MockerFixture -from dangerzone.container_utils import get_runtime_name +from dangerzone.container_utils import Runtime from dangerzone.settings import Settings @@ -10,19 +10,39 @@ def test_get_runtime_name_from_settings(mocker: MockerFixture, tmp_path: Path) - mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) settings = Settings() - settings.set("container_runtime", "new-kid-on-the-block", autosave=True) + settings.set( + "container_runtime", "/opt/somewhere/new-kid-on-the-block", autosave=True + ) - assert get_runtime_name() == "new-kid-on-the-block" + assert Runtime().name == "new-kid-on-the-block" -def test_get_runtime_name_linux(mocker: MockerFixture) -> None: +def test_get_runtime_name_linux(mocker: MockerFixture, tmp_path: Path) -> None: + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) mocker.patch("platform.system", return_value="Linux") - assert get_runtime_name() == "podman" + mocker.patch( + "dangerzone.container_utils.shutil.which", return_value="/usr/bin/podman" + ) + mocker.patch("dangerzone.container_utils.os.path.exists", return_value=True) + runtime = Runtime() + assert runtime.name == "podman" + assert runtime.path == Path("/usr/bin/podman") -def test_get_runtime_name_non_linux(mocker: MockerFixture) -> None: +def test_get_runtime_name_non_linux(mocker: MockerFixture, tmp_path: Path) -> None: mocker.patch("platform.system", return_value="Windows") - assert get_runtime_name() == "docker" + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + mocker.patch( + "dangerzone.container_utils.shutil.which", return_value="/usr/bin/docker" + ) + mocker.patch("dangerzone.container_utils.os.path.exists", return_value=True) + runtime = Runtime() + assert runtime.name == "docker" + assert runtime.path == Path("/usr/bin/docker") mocker.patch("platform.system", return_value="Something else") - assert get_runtime_name() == "docker" + + runtime = Runtime() + assert runtime.name == "docker" + assert runtime.path == Path("/usr/bin/docker") + assert Runtime().name == "docker" From 938c38a40bb27906b53e6e1d039fbf26f6c04d28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Tue, 25 Mar 2025 12:25:24 +0100 Subject: [PATCH 08/13] Update CHANGELOG --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d2c361..a497aa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,15 @@ since 0.4.1, and this project adheres to [Semantic Versioning](https://semver.or - Document Operating System support [#986](https://github.com/freedomofpress/dangerzone/issues/986) - Tests: Look for regressions when converting PDFs [#321](https://github.com/freedomofpress/dangerzone/issues/321) +## Added + +- It is now possible to specify a custom container runtime in the settings, by + using the `container_runtime` key. It should contain the path to the container + runtime you want to use. Please note that this doesn't mean we support more + container runtimes than Podman and Docker for the time being, but enables you to + chose which one you want to use, independently of your platform. + ([#925](https://github.com/freedomofpress/dangerzone/issues/925)) + ## [0.8.1](https://github.com/freedomofpress/dangerzone/compare/v0.8.1...0.8.0) - Update the container image From 53e36ca01b919792cfd85caa452bcd00f5072f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 27 Mar 2025 17:08:32 +0100 Subject: [PATCH 09/13] fixup! Update CHANGELOG --- CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a497aa7..b145f6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,12 +18,12 @@ since 0.4.1, and this project adheres to [Semantic Versioning](https://semver.or ## Added -- It is now possible to specify a custom container runtime in the settings, by - using the `container_runtime` key. It should contain the path to the container - runtime you want to use. Please note that this doesn't mean we support more - container runtimes than Podman and Docker for the time being, but enables you to - chose which one you want to use, independently of your platform. - ([#925](https://github.com/freedomofpress/dangerzone/issues/925)) +- (experimental): It is now possible to specify a custom container runtime in + the settings, by using the `container_runtime` key. It should contain the path + to the container runtime you want to use. Please note that this doesn't mean + we support more container runtimes than Podman and Docker for the time being, + but enables you to chose which one you want to use, independently of your + platform. ([#925](https://github.com/freedomofpress/dangerzone/issues/925)) ## [0.8.1](https://github.com/freedomofpress/dangerzone/compare/v0.8.1...0.8.0) From 2e254ee0fa47d5904ad73471dca17a143d5c4e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 28 Mar 2025 13:30:32 +0100 Subject: [PATCH 10/13] Reset terminal colors after printing the banner --- dangerzone/cli.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dangerzone/cli.py b/dangerzone/cli.py index dfac0ae..5e19fab 100644 --- a/dangerzone/cli.py +++ b/dangerzone/cli.py @@ -320,4 +320,10 @@ def display_banner() -> None: + Style.DIM + "│" ) - print(Back.BLACK + Fore.YELLOW + Style.DIM + "╰──────────────────────────╯") + print( + Back.BLACK + + Fore.YELLOW + + Style.DIM + + "╰──────────────────────────╯" + + Style.RESET_ALL + ) From 73d7a466906bb87e8d5e90dac7d3d83c9fadf67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 28 Mar 2025 13:31:23 +0100 Subject: [PATCH 11/13] Add a `set-container-runtime` option to `dangerzone-cli` This sets the container runtime in the settings, and provides an easy way to do so for users, without having to mess with the json settings. --- dangerzone/cli.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dangerzone/cli.py b/dangerzone/cli.py index 5e19fab..c5ba9d8 100644 --- a/dangerzone/cli.py +++ b/dangerzone/cli.py @@ -11,6 +11,7 @@ from .isolation_provider.container import Container from .isolation_provider.dummy import Dummy from .isolation_provider.qubes import Qubes, is_qubes_native_conversion from .logic import DangerzoneCore +from .settings import Settings from .util import get_version, replace_control_chars @@ -48,6 +49,11 @@ def print_header(s: str) -> None: flag_value=True, help="Run Dangerzone in debug mode, to get logs from gVisor.", ) +@click.option( + "--set-container-runtime", + required=False, + help="The path to the container runtime you want to set in the settings", +) @click.version_option(version=get_version(), message="%(version)s") @errors.handle_document_errors def cli_main( @@ -57,8 +63,14 @@ def cli_main( archive: bool, dummy_conversion: bool, debug: bool, + set_container_runtime: Optional[str] = None, ) -> None: setup_logging() + display_banner() + if set_container_runtime: + settings = Settings() + settings.set("container_runtime", set_container_runtime, autosave=True) + click.echo(f"Set the settings container_runtime to {set_container_runtime}") if getattr(sys, "dangerzone_dev", False) and dummy_conversion: dangerzone = DangerzoneCore(Dummy()) @@ -67,7 +79,6 @@ def cli_main( else: dangerzone = DangerzoneCore(Container(debug=debug)) - display_banner() if len(filenames) == 1 and output_filename: dangerzone.add_document_from_filename(filenames[0], output_filename, archive) elif len(filenames) > 1 and output_filename: From 2a29cf7c27b53df43437e8cb6e88b31ad2675fad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 28 Mar 2025 14:19:54 +0100 Subject: [PATCH 12/13] Ensure that only podman and docker container runtimes can be used --- dangerzone/container_utils.py | 5 +++++ dangerzone/errors.py | 4 ++++ tests/test_container_utils.py | 20 ++++++++++++++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/dangerzone/container_utils.py b/dangerzone/container_utils.py index c645358..6101cdd 100644 --- a/dangerzone/container_utils.py +++ b/dangerzone/container_utils.py @@ -21,6 +21,8 @@ class Runtime(object): if settings.custom_runtime_specified(): self.path = Path(settings.get("container_runtime")) + if not self.path.exists(): + raise errors.UnsupportedContainerRuntime(self.path) self.name = self.path.stem else: self.name = self.get_default_runtime_name() @@ -29,6 +31,9 @@ class Runtime(object): raise errors.NoContainerTechException(self.name) self.path = Path(binary_path) + if self.name not in ("podman", "docker"): + raise errors.UnsupportedContainerRuntime(self.name) + @staticmethod def get_default_runtime_name() -> str: return "podman" if platform.system() == "Linux" else "docker" diff --git a/dangerzone/errors.py b/dangerzone/errors.py index d8e1759..c1c2849 100644 --- a/dangerzone/errors.py +++ b/dangerzone/errors.py @@ -140,3 +140,7 @@ class NotAvailableContainerTechException(Exception): self.error = error self.container_tech = container_tech super().__init__(f"{container_tech} is not available") + + +class UnsupportedContainerRuntime(Exception): + pass diff --git a/tests/test_container_utils.py b/tests/test_container_utils.py index 570bae1..e7ee07e 100644 --- a/tests/test_container_utils.py +++ b/tests/test_container_utils.py @@ -1,20 +1,21 @@ from pathlib import Path +import pytest from pytest_mock import MockerFixture +from dangerzone import errors from dangerzone.container_utils import Runtime from dangerzone.settings import Settings def test_get_runtime_name_from_settings(mocker: MockerFixture, tmp_path: Path) -> None: mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + mocker.patch("dangerzone.container_utils.Path.exists", return_value=True) settings = Settings() - settings.set( - "container_runtime", "/opt/somewhere/new-kid-on-the-block", autosave=True - ) + settings.set("container_runtime", "/opt/somewhere/docker", autosave=True) - assert Runtime().name == "new-kid-on-the-block" + assert Runtime().name == "docker" def test_get_runtime_name_linux(mocker: MockerFixture, tmp_path: Path) -> None: @@ -46,3 +47,14 @@ def test_get_runtime_name_non_linux(mocker: MockerFixture, tmp_path: Path) -> No assert runtime.name == "docker" assert runtime.path == Path("/usr/bin/docker") assert Runtime().name == "docker" + + +def test_get_unsupported_runtime_name(mocker: MockerFixture, tmp_path: Path): + mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) + settings = Settings() + settings.set( + "container_runtime", "/opt/somewhere/new-kid-on-the-block", autosave=True + ) + + with pytest.raises(errors.UnsupportedContainerRuntime): + assert Runtime().name == "new-kid-on-the-block" From 6c8a75732e56479a435a014f113b033251b53817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Fri, 28 Mar 2025 14:24:30 +0100 Subject: [PATCH 13/13] FIXUP: return type for mypy --- tests/test_container_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_container_utils.py b/tests/test_container_utils.py index e7ee07e..db47b5a 100644 --- a/tests/test_container_utils.py +++ b/tests/test_container_utils.py @@ -49,7 +49,7 @@ def test_get_runtime_name_non_linux(mocker: MockerFixture, tmp_path: Path) -> No assert Runtime().name == "docker" -def test_get_unsupported_runtime_name(mocker: MockerFixture, tmp_path: Path): +def test_get_unsupported_runtime_name(mocker: MockerFixture, tmp_path: Path) -> None: mocker.patch("dangerzone.settings.get_config_dir", return_value=tmp_path) settings = Settings() settings.set(