Display the {podman,docker} pull progress when installing a new image

The progressbars we see when using this same commands on the
command line doesn't seem to be passed to the python process here,
unfortunately.

dangerzone/container_utils.py

@@ -3,13 +3,13 @@ import logging
 import platform
 import shutil
 import subprocess
-from typing import List, Optional, Tuple
+from typing import IO, Callable, List, Optional, Tuple
 
 from . import errors
 from .util import get_resource_path, get_subprocess_startupinfo
 
 OLD_CONTAINER_NAME = "dangerzone.rocks/dangerzone"
-CONTAINER_NAME = "ghcr.io/freedomofpress/dangerzone/dangerzone"
+CONTAINER_NAME = "ghcr.io/almet/dangerzone/dangerzone"  # FIXME: Change this to the correct container name
 
 log = logging.getLogger(__name__)
 
@@ -180,15 +180,26 @@ def get_image_id_by_digest(digest: str) -> str:
     return process.stdout.decode().strip().split("\n")[0]
 
 
-def container_pull(image: str, manifest_digest: str):
+def container_pull(image: str, manifest_digest: str, callback: Callable):
     """Pull a container image from a registry."""
     cmd = [get_runtime_name(), "pull", f"{image}@sha256:{manifest_digest}"]
-    try:
+    process = subprocess.Popen(
-        subprocess_run(cmd, check=True)
+        cmd,
-    except subprocess.CalledProcessError as e:
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        text=True,
+        bufsize=1,
+        universal_newlines=True,
+    )
+
+    for line in process.stdout:  # type: ignore
+        callback(line)
+
+    process.wait()
+    if process.returncode != 0:
         raise errors.ContainerPullException(
-            f"Could not pull the container image: {e}"
+            f"Could not pull the container image: {process.returncode}"
-        ) from e
+        )
 
 
 def get_local_image_digest(image: str) -> str:

dangerzone/gui/main_window.py

@@ -1,3 +1,4 @@
+import io
 import logging
 import os
 import platform
@@ -5,22 +6,24 @@ import tempfile
 import typing
 from multiprocessing.pool import ThreadPool
 from pathlib import Path
-from typing import List, Optional
+from typing import Callable, List, Optional
 
 # FIXME: See https://github.com/freedomofpress/dangerzone/issues/320 for more details.
 if typing.TYPE_CHECKING:
     from PySide2 import QtCore, QtGui, QtSvg, QtWidgets
     from PySide2.QtCore import Qt
+    from PySide2.QtGui import QTextCursor
     from PySide2.QtWidgets import QAction, QTextEdit
 else:
     try:
         from PySide6 import QtCore, QtGui, QtSvg, QtWidgets
         from PySide6.QtCore import Qt
-        from PySide6.QtGui import QAction
+        from PySide6.QtGui import QAction, QTextCursor
         from PySide6.QtWidgets import QTextEdit
     except ImportError:
         from PySide2 import QtCore, QtGui, QtSvg, QtWidgets
         from PySide2.QtCore import Qt
+        from PySide2.QtGui import QTextCursor
         from PySide2.QtWidgets import QAction, QTextEdit
 
 from .. import errors
@@ -171,7 +174,7 @@ class MainWindow(QtWidgets.QMainWindow):
         self.toggle_updates_action.triggered.connect(self.toggle_updates_triggered)
         self.toggle_updates_action.setCheckable(True)
         self.toggle_updates_action.setChecked(
-            bool(self.dangerzone.settings.get("updater_check"))
+            bool(self.dangerzone.settings.get("updater_check_all"))
         )
 
         # Add the "Exit" action
@@ -284,7 +287,7 @@ class MainWindow(QtWidgets.QMainWindow):
     def toggle_updates_triggered(self) -> None:
         """Change the underlying update check settings based on the user's choice."""
         check = self.toggle_updates_action.isChecked()
-        self.dangerzone.settings.set("updater_check", check)
+        self.dangerzone.settings.set("updater_check_all", check)
         self.dangerzone.settings.save()
 
     def handle_docker_desktop_version_check(
@@ -439,15 +442,21 @@ class MainWindow(QtWidgets.QMainWindow):
 
 class InstallContainerThread(QtCore.QThread):
     finished = QtCore.Signal(str)
+    process_stdout = QtCore.Signal(str)
 
-    def __init__(self, dangerzone: DangerzoneGui) -> None:
+    def __init__(
+        self, dangerzone: DangerzoneGui, callback: Optional[Callable] = None
+    ) -> None:
         super(InstallContainerThread, self).__init__()
         self.dangerzone = dangerzone
 
     def run(self) -> None:
         error = None
         try:
-            installed = self.dangerzone.isolation_provider.install()
+            should_upgrade = self.dangerzone.settings.get("updater_check_all")
+            installed = self.dangerzone.isolation_provider.install(
+                should_upgrade=bool(should_upgrade), callback=self.process_stdout.emit
+            )
         except Exception as e:
             log.error("Container installation problem")
             error = format_exception(e)
@@ -482,11 +491,20 @@ class TracebackWidget(QTextEdit):
         # Enable copying
         self.setTextInteractionFlags(Qt.TextSelectableByMouse)
 
+        self.current_output = ""
+
     def set_content(self, error: Optional[str] = None) -> None:
         if error:
             self.setPlainText(error)
             self.setVisible(True)
 
+    def process_output(self, line):
+        self.current_output += line
+        self.setText(self.current_output)
+        cursor = self.textCursor()
+        cursor.movePosition(QTextCursor.MoveOperation.End)
+        self.setTextCursor(cursor)
+
 
 class WaitingWidgetContainer(WaitingWidget):
     # These are the possible states that the WaitingWidget can show.
@@ -613,8 +631,14 @@ class WaitingWidgetContainer(WaitingWidget):
                 "Installing the Dangerzone container image.<br><br>"
                 "This might take a few minutes..."
             )
+            self.traceback.setVisible(True)
+
             self.install_container_t = InstallContainerThread(self.dangerzone)
             self.install_container_t.finished.connect(self.installation_finished)
+
+            self.install_container_t.process_stdout.connect(
+                self.traceback.process_output
+            )
             self.install_container_t.start()
 
 

dangerzone/gui/updater.py

@@ -106,7 +106,7 @@ class UpdaterThread(QtCore.QThread):
             should_check = self.prompt_for_checks()
             if should_check is not None:
                 self.dangerzone.settings.set(
-                    "updater_check", should_check, autosave=True
+                    "updater_check_all", should_check, autosave=True
                 )
         return bool(should_check)
 

dangerzone/isolation_provider/base.py

@@ -95,7 +95,7 @@ class IsolationProvider(ABC):
         return self.debug or getattr(sys, "dangerzone_dev", False)
 
     @abstractmethod
-    def install(self) -> bool:
+    def install(self, should_upgrade: bool, callback: Callable) -> bool:
         pass
 
     def convert(

dangerzone/isolation_provider/container.py

@@ -3,7 +3,7 @@ import os
 import platform
 import shlex
 import subprocess
-from typing import List, Tuple
+from typing import Callable, List, Tuple
 
 from .. import container_utils, errors, updater
 from ..document import Document
@@ -77,25 +77,38 @@ class Container(IsolationProvider):
         return security_args
 
     @staticmethod
-    def install() -> bool:
+    def install(
+        should_upgrade: bool, callback: Callable, last_try: bool = False
+    ) -> bool:
         """Check if an update is available and install it if necessary."""
-        # XXX Do this only if users have opted in to auto-updates
+        if not should_upgrade:
-
+            log.debug("Skipping container upgrade check as requested by the settings")
-        if False:  # Comment this for now, just as an exemple of this can be implemented
+        else:
-            # # Load the image tarball into the container runtime.
             update_available, image_digest = updater.is_update_available(
-                container_utils.CONTAINER_NAME
+                container_utils.CONTAINER_NAME,
+                updater.DEFAULT_PUBKEY_LOCATION,
             )
             if update_available and image_digest:
+                log.debug("Upgrading container image to %s", image_digest)
                 updater.upgrade_container_image(
                     container_utils.CONTAINER_NAME,
                     image_digest,
                     updater.DEFAULT_PUBKEY_LOCATION,
+                    callback=callback,
                 )
-
+            else:
-        updater.verify_local_image(
+                log.debug("No update available for the container")
-            container_utils.CONTAINER_NAME, updater.DEFAULT_PUBKEY_LOCATION
+        try:
-        )
+            updater.verify_local_image(
+                container_utils.CONTAINER_NAME, updater.DEFAULT_PUBKEY_LOCATION
+            )
+        except errors.ImageNotPresentException:
+            if last_try:
+                raise
+            log.debug("Container image not found, trying to install it.")
+            return Container.install(
+                should_upgrade=should_upgrade, callback=callback, last_try=True
+            )
 
         return True
 

dangerzone/settings.py

@@ -1,7 +1,6 @@
 import json
 import logging
 import os
-import platform
 from typing import TYPE_CHECKING, Any, Dict
 
 from packaging import version
@@ -38,7 +37,7 @@ class Settings:
             "open": True,
             "open_app": None,
             "safe_extension": SAFE_EXTENSION,
-            "updater_check": None,
+            "updater_check_all": None,
             "updater_last_check": None,  # last check in UNIX epoch (secs since 1970)
             # FIXME: How to invalidate those if they change upstream?
             "updater_latest_version": get_version(),

dangerzone/updater/cli.py

@@ -42,6 +42,17 @@ def upgrade(image: str, pubkey: str) -> None:
         raise click.Abort()
 
 
+@main.command()
+@click.argument("image", default=DEFAULT_IMAGE_NAME)
+@click.option("--pubkey", default=signatures.DEFAULT_PUBKEY_LOCATION)
+def store_signatures(image: str, pubkey: str) -> None:
+    manifest_digest = registry.get_manifest_digest(image)
+    sigs = signatures.get_remote_signatures(image, manifest_digest)
+    signatures.verify_signatures(sigs, manifest_digest, pubkey)
+    signatures.store_signatures(sigs, manifest_digest, pubkey, update_logindex=False)
+    click.echo(f"✅ Signatures has been verified and stored locally")
+
+
 @main.command()
 @click.argument("image_filename")
 @click.option("--pubkey", default=signatures.DEFAULT_PUBKEY_LOCATION)

dangerzone/updater/releases.py

@@ -114,14 +114,14 @@ def should_check_for_releases(settings: Settings) -> bool:
     * Thus we will always show to the user the cached info about the new version,
         and won't make a new update check.
     """
-    check = settings.get("updater_check")
+    check = settings.get("updater_check_all")
 
     log.debug("Checking platform type")
     # TODO: Disable updates for Homebrew installations.
     if platform.system() == "Linux" and not getattr(sys, "dangerzone_dev", False):
         log.debug("Running on Linux, disabling updates")
         if not check:  # if not overidden by user
-            settings.set("updater_check", False, autosave=True)
+            settings.set("updater_check_all", False, autosave=True)
             return False
 
     log.debug("Checking if first run of Dangerzone")

dangerzone/updater/signatures.py

@@ -9,7 +9,7 @@ from hashlib import sha256
 from io import BytesIO
 from pathlib import Path
 from tempfile import NamedTemporaryFile, TemporaryDirectory
-from typing import Dict, List, Optional, Tuple
+from typing import Callable, Dict, List, Optional, Tuple
 
 from .. import container_utils as runtime
 from .. import errors as dzerrors
@@ -369,7 +369,9 @@ def load_and_verify_signatures(
     return signatures
 
 
-def store_signatures(signatures: list[Dict], image_digest: str, pubkey: str) -> None:
+def store_signatures(
+    signatures: list[Dict], image_digest: str, pubkey: str, update_logindex: bool = True
+) -> None:
     """
     Store signatures locally in the SIGNATURE_PATH folder, like this:
 
@@ -414,7 +416,8 @@ def store_signatures(signatures: list[Dict], image_digest: str, pubkey: str) ->
         )
         json.dump(signatures, f)
 
-    write_log_index(get_log_index_from_signatures(signatures))
+    if update_logindex:
+        write_log_index(get_log_index_from_signatures(signatures))
 
 
 def verify_local_image(image: str, pubkey: str) -> bool:
@@ -478,14 +481,16 @@ def prepare_airgapped_archive(image_name: str, destination: str) -> None:
             archive.add(tmpdir, arcname=".")
 
 
-def upgrade_container_image(image: str, manifest_digest: str, pubkey: str) -> str:
+def upgrade_container_image(
+    image: str, manifest_digest: str, pubkey: str, callback: Callable
+) -> str:
     """Verify and upgrade the image to the latest, if signed."""
     update_available, remote_digest = registry.is_new_remote_image_available(image)
     if not update_available:
         raise errors.ImageAlreadyUpToDate("The image is already up to date")
 
     signatures = check_signatures_and_logindex(image, remote_digest, pubkey)
-    runtime.container_pull(image, manifest_digest)
+    runtime.container_pull(image, manifest_digest, callback=callback)
 
     # Store the signatures just now to avoid storing them unverified
     store_signatures(signatures, manifest_digest, pubkey)

docs/developer/updates.md

@@ -11,8 +11,7 @@ https://github.com/freedomofpress/dangerzone/wiki/Updates
 
 ## Design overview
 
-This feature introduces a hamburger icon that will be visible across almost all
+A hamburger icon is visible across almost all of the Dangerzone windows, and is used to notify the users when there are new releases.
-of the Dangerzone windows. This will be used to notify the users about updates.
 
 ### First run
 
@@ -21,8 +20,7 @@ _We detect it's the first time Dangerzone runs because the
 
 Add the following keys in our `settings.json` file.
 
-* `"updater_check": None`: Whether to check for updates or not. `None` means
+* `"updater_check_all": True`: Whether or not to check and apply independent container updates and check for new releases.
-  that the user has not decided yet, and is the default.
 * `"updater_last_check": None`: The last time we checked for updates (in seconds
   from Unix epoch). None means that we haven't checked yet.
 * `"updater_latest_version": "0.4.2"`: The latest version that the Dangerzone
@@ -32,43 +30,19 @@ Add the following keys in our `settings.json` file.
 * `"updater_errors: 0`: The number of update check errors that we have
   encountered in a row.
 
-Note:
+Previously, `"updater_check"` was used to determine if we should check for new releases, and has been replaced by `"updater_check_all"` when adding support for independent container updates.
-
-* If on Linux, make `"updater_check": False`, since we normally have
-  other update channels for these platforms.
 
 ### Second run
 
 _We detect it's the second time Dangerzone runs because
-`settings["updater_check"] is not None and settings["updater_last_check"] is
+`settings["updater_check_all"] is not None and settings["updater_last_check"] is
 None`._
 
-Before starting up the main window, show this window:
+Before starting up the main window, the user is prompted if they want to enable update checks.
-
-* Title: Dangerzone Updater
-* Body:
-
-  > Do you want Dangerzone to automatically check for updates?
-  >
-  > If you accept, Dangerzone will check the latest releases page in github.com
-  > on startup. Otherwise it will make no network requests and won't inform you
-  > about new releases.
-  >
-  > If you prefer another way of getting notified about new releases, we suggest adding
-  > to your RSS reader our [Mastodon feed](https://fosstodon.org/@dangerzone.rss). For more information
-  > about updates, check [this webpage](https://github.com/freedomofpress/dangerzone/wiki/Updates).
-
-* Buttons:
-   - Check Automaticaly: Store `settings["updater_check"] = True`
-   - Don't Check: Store `settings["updater_check"] = False`
-
-Note:
-* Users will be able to change their choice from the hamburger menu, which will
-  contain an entry called "Check for updates", that users can check and uncheck.
 
 ### Subsequent runs
 
-_We perform the following only if `settings["updater_check"] == True`._
+_We perform the following only if `settings["updater_check_all"] == True`._
 
 1. Spawn a new thread so that we don't block the main window.
 2. Check if we have cached information about a release (version and changelog).

tests/gui/test_main_window.py

@@ -97,7 +97,7 @@ def test_default_menu(
     updater: UpdaterThread,
 ) -> None:
     """Check that the default menu entries are in order."""
-    updater.dangerzone.settings.set("updater_check", True)
+    updater.dangerzone.settings.set("updater_check_all", True)
 
     window = MainWindow(updater.dangerzone)
     menu_actions = window.hamburger_button.menu().actions()
@@ -115,7 +115,7 @@ def test_default_menu(
 
     toggle_updates_action.trigger()
     assert not toggle_updates_action.isChecked()
-    assert updater.dangerzone.settings.get("updater_check") is False
+    assert updater.dangerzone.settings.get("updater_check_all") is False
 
 
 def test_no_update(
@@ -128,12 +128,12 @@ def test_no_update(
     # Check that when no update is detected, e.g., due to update cooldown, an empty
     # report is received that does not affect the menu entries.
     curtime = int(time.time())
-    updater.dangerzone.settings.set("updater_check", True)
+    updater.dangerzone.settings.set("updater_check_all", True)
     updater.dangerzone.settings.set("updater_errors", 9)
     updater.dangerzone.settings.set("updater_last_check", curtime)
 
     expected_settings = default_updater_settings()
-    expected_settings["updater_check"] = True
+    expected_settings["updater_check_all"] = True
     expected_settings["updater_errors"] = 0  # errors must be cleared
     expected_settings["updater_last_check"] = curtime
 
@@ -166,7 +166,7 @@ def test_update_detected(
 ) -> None:
     """Test that a newly detected version leads to a notification to the user."""
 
-    qt_updater.dangerzone.settings.set("updater_check", True)
+    qt_updater.dangerzone.settings.set("updater_check_all", True)
     qt_updater.dangerzone.settings.set("updater_last_check", 0)
     qt_updater.dangerzone.settings.set("updater_errors", 9)
 
@@ -198,7 +198,7 @@ def test_update_detected(
 
     # Check that the settings have been updated properly.
     expected_settings = default_updater_settings()
-    expected_settings["updater_check"] = True
+    expected_settings["updater_check_all"] = True
     expected_settings["updater_last_check"] = qt_updater.dangerzone.settings.get(
         "updater_last_check"
     )
@@ -279,7 +279,7 @@ def test_update_error(
 ) -> None:
     """Test that an error during an update check leads to a notification to the user."""
     # Test 1 - Check that the first error does not notify the user.
-    qt_updater.dangerzone.settings.set("updater_check", True)
+    qt_updater.dangerzone.settings.set("updater_check_all", True)
     qt_updater.dangerzone.settings.set("updater_last_check", 0)
     qt_updater.dangerzone.settings.set("updater_errors", 0)
 
@@ -306,7 +306,7 @@ def test_update_error(
 
     # Check that the settings have been updated properly.
     expected_settings = default_updater_settings()
-    expected_settings["updater_check"] = True
+    expected_settings["updater_check_all"] = True
     expected_settings["updater_last_check"] = qt_updater.dangerzone.settings.get(
         "updater_last_check"
     )

tests/gui/test_updater.py

@@ -110,7 +110,7 @@ def test_post_0_4_2_settings(
 def test_linux_no_check(updater: UpdaterThread, monkeypatch: MonkeyPatch) -> None:
     """Ensure that Dangerzone on Linux does not make any update check."""
     expected_settings = default_updater_settings()
-    expected_settings["updater_check"] = False
+    expected_settings["updater_check_all"] = False
     expected_settings["updater_last_check"] = None
 
     # XXX: Simulate Dangerzone installed via package manager.
@@ -130,7 +130,7 @@ def test_user_prompts(
     # When Dangerzone runs for the first time, users should not be asked to enable
     # updates.
     expected_settings = default_updater_settings()
-    expected_settings["updater_check"] = None
+    expected_settings["updater_check_all"] = None
     expected_settings["updater_last_check"] = 0
     assert updater.should_check_for_updates() is False
     assert settings.get_updater_settings() == expected_settings
@@ -145,14 +145,14 @@ def test_user_prompts(
 
     # Check disabling update checks.
     prompt_mock().launch.return_value = False  # type: ignore [attr-defined]
-    expected_settings["updater_check"] = False
+    expected_settings["updater_check_all"] = False
     assert updater.should_check_for_updates() is False
     assert settings.get_updater_settings() == expected_settings
 
-    # Reset the "updater_check" field and check enabling update checks.
+    # Reset the "updater_check_all" field and check enabling update checks.
-    settings.set("updater_check", None)
+    settings.set("updater_check_all", None)
     prompt_mock().launch.return_value = True  # type: ignore [attr-defined]
-    expected_settings["updater_check"] = True
+    expected_settings["updater_check_all"] = True
     assert updater.should_check_for_updates() is True
     assert settings.get_updater_settings() == expected_settings
 
@@ -162,7 +162,7 @@ def test_user_prompts(
     # checks.
     prompt_mock().side_effect = RuntimeError("Should not be called")  # type: ignore [attr-defined]
     for check in [True, False]:
-        settings.set("updater_check", check)
+        settings.set("updater_check_all", check)
         assert updater.should_check_for_updates() == check
 
 
@@ -217,7 +217,7 @@ def test_update_checks_cooldown(updater: UpdaterThread, mocker: MockerFixture) -
     """Make sure Dangerzone only checks for updates every X hours"""
     settings = updater.dangerzone.settings
 
-    settings.set("updater_check", True)
+    settings.set("updater_check_all", True)
     settings.set("updater_last_check", 0)
 
     # Mock some functions before the tests start
@@ -401,7 +401,7 @@ def test_update_check_prompt(
 
     # Test 2 - Check that when the user chooses to enable update checks, we
     # store that decision in the settings.
-    settings.set("updater_check", None, autosave=True)
+    settings.set("updater_check_all", None, autosave=True)
 
     def click_ok() -> None:
         dialog = qt_updater.dangerzone.app.activeWindow()
@@ -411,11 +411,11 @@ def test_update_check_prompt(
     res = qt_updater.should_check_for_updates()
 
     assert res is True
-    assert settings.get("updater_check") is True
+    assert settings.get("updater_check_all") is True
 
     # Test 3 - Same as the previous test, but check that clicking on cancel stores the
     # opposite decision.
-    settings.set("updater_check", None)  # type: ignore [unreachable]
+    settings.set("updater_check_all", None)  # type: ignore [unreachable]
 
     def click_cancel() -> None:
         dialog = qt_updater.dangerzone.app.activeWindow()
@@ -425,11 +425,11 @@ def test_update_check_prompt(
     res = qt_updater.should_check_for_updates()
 
     assert res is False
-    assert settings.get("updater_check") is False
+    assert settings.get("updater_check_all") is False
 
     # Test 4 - Same as the previous test, but check that clicking on "X" does not store
     # any decision.
-    settings.set("updater_check", None, autosave=True)
+    settings.set("updater_check_all", None, autosave=True)
 
     def click_x() -> None:
         dialog = qt_updater.dangerzone.app.activeWindow()
@@ -439,4 +439,4 @@ def test_update_check_prompt(
     res = qt_updater.should_check_for_updates()
 
     assert res is False
-    assert settings.get("updater_check") is None
+    assert settings.get("updater_check_all") is None