Merge 57973b8a2e into fdc27c4d3b

Revamp the container image installation process in a way that does not
involve using image IDs. We don't want to rely on image IDs anymore,
since they are brittle (see
https://github.com/freedomofpress/dangerzone/issues/933). Instead, we
use image tags, as provided in the `image-id.txt` file.  This allows us
to check fast if an image is up to date, and we no longer need to
maintain multiple image IDs from various container runtimes.

Refs #933
Refs #988
Fixes #1020

.github/workflows/build.yml

@@ -74,6 +74,8 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
 
       - name: Get current date
         id: date

.github/workflows/ci.yml

@@ -48,6 +48,8 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
 
       - name: Get current date
         id: date

.github/workflows/scan.yml

@@ -14,6 +14,8 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
       - name: Install container build dependencies
         run: sudo apt install pipx && pipx install poetry
       - name: Build container image

RELEASE.md

@@ -141,9 +141,10 @@ Close the Dangerzone application and get the container image for that
 version. For example:
 
 ```
-$ docker images dangerzone.rocks/dangerzone:latest
+$ docker images dangerzone.rocks/dangerzone
 REPOSITORY                   TAG         IMAGE ID      CREATED       SIZE
 dangerzone.rocks/dangerzone  latest      <image ID>    <date>        <size>
+dangerzone.rocks/dangerzone  <tag>       <image ID>    <date>        <size>
 ```
 
 Then run the version under QA and ensure that the settings remain changed.
@@ -152,9 +153,10 @@ Afterwards check that new docker image was installed by running the same command
 and seeing the following differences:
 
 ```
-$ docker images dangerzone.rocks/dangerzone:latest
+$ docker images dangerzone.rocks/dangerzone
 REPOSITORY                   TAG         IMAGE ID        CREATED       SIZE
 dangerzone.rocks/dangerzone  latest      <different ID>  <newer date>  <different size>
+dangerzone.rocks/dangerzone  <other tag> <different ID>  <newer date>  <different size>
 ```
 
 #### 4. Dangerzone successfully installs the container image
@@ -280,7 +282,7 @@ Once we are confident that the release will be out shortly, and doesn't need any
   ```
   Then copy the `share/container.tar.gz` to the assets folder on `dangerzone-$VERSION-arm64.tar.gz`, along with the `share/image-id.txt` file.
 - [ ] Run `poetry run ./install/macos/build-app.py`; this will make `dist/Dangerzone.app`
-- [ ] Make sure that the build application works with the containerd graph
+- [ ] Make sure that the built application works with the containerd graph
   driver (see [#933](https://github.com/freedomofpress/dangerzone/issues/933))
 - [ ] Run `poetry run ./install/macos/build-app.py --only-codesign`; this will make `dist/Dangerzone.dmg`
   * You need to run this command as the account that has access to the code signing certificate
@@ -326,9 +328,6 @@ The Windows release is performed in a Windows 11 virtual machine as opposed to a
 - [ ] Copy the container image into the VM
   > [!IMPORTANT]
   > Instead of running `python .\install\windows\build-image.py` in the VM, run the build image script on the host (making sure to build for `linux/amd64`). Copy `share/container.tar.gz` and `share/image-id.txt` from the host into the `share` folder in the VM.
-  > Also, don't forget to add the supplementary image ID (see
-  > [#933](https://github.com/freedomofpress/dangerzone/issues/933)) in
-  > `share/image-id.txt`)
 - [ ] Run `poetry run .\install\windows\build-app.bat`
 - [ ] When you're done you will have `dist\Dangerzone.msi`
 

dangerzone/gui/main_window.py

@@ -500,6 +500,7 @@ class WaitingWidgetContainer(WaitingWidget):
         error: Optional[str] = None
 
         try:
+            assert isinstance(self.dangerzone.isolation_provider, (Dummy, Container))
             self.dangerzone.isolation_provider.is_runtime_available()
         except NoContainerTechException as e:
             log.error(str(e))

dangerzone/isolation_provider/base.py

@@ -93,10 +93,6 @@ class IsolationProvider(ABC):
         else:
             self.proc_stderr = subprocess.DEVNULL
 
-    @staticmethod
-    def is_runtime_available() -> bool:
-        return True
-
     @abstractmethod
     def install(self) -> bool:
         pass

dangerzone/isolation_provider/container.py

@@ -1,11 +1,12 @@
 import gzip
+import json
 import logging
 import os
 import platform
 import shlex
 import shutil
 import subprocess
-from typing import List, Tuple
+from typing import Dict, List, Tuple
 
 from ..document import Document
 from ..util import get_resource_path, get_subprocess_startupinfo
@@ -155,16 +156,81 @@ class Container(IsolationProvider):
         return security_args
 
     @staticmethod
-    def install() -> bool:
+    def list_image_tags() -> Dict[str, str]:
-        """
+        """Get the tags of all loaded Dangerzone images.
-        Make sure the podman container is installed. Linux only.
-        """
-        if Container.is_container_installed():
-            return True
 
-        # Load the container into podman
+        This method returns a mapping of image tags to image IDs, for all Dangerzone
+        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.
+        """
+        images = json.loads(
+            subprocess.check_output(
+                [
+                    Container.get_runtime(),
+                    "image",
+                    "list",
+                    "--format",
+                    "json",
+                    Container.CONTAINER_NAME,
+                ],
+                text=True,
+                startupinfo=get_subprocess_startupinfo(),
+            )
+        )
+
+        # Grab every image name and associate it with an image ID.
+        tags = {}
+        for image in images:
+            for name in image["Names"]:
+                tag = name.split(":")[1]
+                tags[tag] = image["Id"]
+
+        return tags
+
+    @staticmethod
+    def delete_image_tag(tag: str) -> None:
+        """Delete a Dangerzone image tag."""
+        name = Container.CONTAINER_NAME + ":" + tag
+        log.warning(f"Deleting old container image: {name}")
+        try:
+            subprocess.check_output(
+                [Container.get_runtime(), "rmi", "--force", name],
+                startupinfo=get_subprocess_startupinfo(),
+            )
+        except Exception as e:
+            log.warning(
+                f"Couldn't delete old container image '{name}', so leaving it there."
+                f" Original error: {e}"
+            )
+
+    @staticmethod
+    def add_image_tag(cur_tag: str, new_tag: str) -> None:
+        """Add a tag to an existing Dangerzone image."""
+        cur_image_name = Container.CONTAINER_NAME + ":" + cur_tag
+        new_image_name = Container.CONTAINER_NAME + ":" + new_tag
+        subprocess.check_output(
+            [
+                Container.get_runtime(),
+                "tag",
+                cur_image_name,
+                new_image_name,
+            ],
+            startupinfo=get_subprocess_startupinfo(),
+        )
+
+        log.info(
+            f"Successfully tagged container image '{cur_image_name}' as '{new_image_name}'"
+        )
+
+    @staticmethod
+    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:
+            return f.read().strip()
+
+    @staticmethod
+    def load_image_tarball() -> None:
         log.info("Installing Dangerzone container image...")
-
         p = subprocess.Popen(
             [Container.get_runtime(), "load"],
             stdin=subprocess.PIPE,
@@ -191,10 +257,54 @@ class Container(IsolationProvider):
                 f"Could not install container image: {error}"
             )
 
-        if not Container.is_container_installed(raise_on_error=True):
+        log.info("Successfully installed container image from")
-            return False
 
-        log.info("Container image installed")
+    @staticmethod
+    def install() -> bool:
+        """Install the container image tarball, or verify that it's already installed.
+
+        Perform the following actions:
+        1. Get the tags of any locally available images that match Dangerzone's image
+           name.
+        2. Get the expected image tag from the image-id.txt file.
+           - If this tag is present in the local images, and that image is also tagged
+             as "latest", then we can return.
+           - Else, prune the older container images and continue.
+        3. Load the image tarball and make sure it matches the expected tag.
+        4. Tag that image as "latest", and mark the installation as finished.
+        """
+        old_tags = Container.list_image_tags()
+        expected_tag = Container.get_expected_tag()
+
+        if expected_tag not in old_tags:
+            # Prune older container images.
+            log.info(
+                f"Could not find a Dangerzone container image with tag '{expected_tag}'"
+            )
+            for tag in old_tags.keys():
+                Container.delete_image_tag(tag)
+        elif old_tags[expected_tag] != old_tags.get("latest"):
+            log.info(f"The expected tag '{expected_tag}' is not the latest one")
+            Container.add_image_tag(expected_tag, "latest")
+            return True
+        else:
+            return True
+
+        # Load the image tarball into the container runtime.
+        Container.load_image_tarball()
+
+        # Check that the container image has the expected image tag.
+        # See https://github.com/freedomofpress/dangerzone/issues/988 for an example
+        # where this was not the case.
+        new_tags = Container.list_image_tags()
+        if expected_tag not in new_tags:
+            raise ImageNotPresentException(
+                f"Could not find expected tag '{expected_tag}' after loading the"
+                " container image tarball"
+            )
+
+        # Mark the expected tag as "latest".
+        Container.add_image_tag(expected_tag, "latest")
         return True
 
     @staticmethod
@@ -213,58 +323,6 @@ class Container(IsolationProvider):
                 raise NotAvailableContainerTechException(runtime_name, stderr.decode())
             return True
 
-    @staticmethod
-    def is_container_installed(raise_on_error: bool = False) -> bool:
-        """
-        See if the container is installed.
-        """
-        # Get the image id
-        with open(get_resource_path("image-id.txt")) as f:
-            expected_image_ids = f.read().strip().split()
-
-        # See if this image is already installed
-        installed = False
-        found_image_id = subprocess.check_output(
-            [
-                Container.get_runtime(),
-                "image",
-                "list",
-                "--format",
-                "{{.ID}}",
-                Container.CONTAINER_NAME,
-            ],
-            text=True,
-            startupinfo=get_subprocess_startupinfo(),
-        )
-        found_image_id = found_image_id.strip()
-
-        if found_image_id in expected_image_ids:
-            installed = True
-        elif found_image_id == "":
-            if raise_on_error:
-                raise ImageNotPresentException(
-                    "Image is not listed after installation. Bailing out."
-                )
-        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 ImageNotPresentException(msg)
-            log.info(msg)
-            log.info("Deleting old dangerzone container image")
-
-            try:
-                subprocess.check_output(
-                    [Container.get_runtime(), "rmi", "--force", found_image_id],
-                    startupinfo=get_subprocess_startupinfo(),
-                )
-            except Exception:
-                log.warning("Couldn't delete old container image, so leaving it there")
-
-        return installed
-
     def doc_to_pixels_container_name(self, document: Document) -> str:
         """Unique container name for the doc-to-pixels phase."""
         return f"dangerzone-doc-to-pixels-{document.id}"

dangerzone/isolation_provider/dummy.py

@@ -39,6 +39,10 @@ class Dummy(IsolationProvider):
     def install(self) -> bool:
         return True
 
+    @staticmethod
+    def is_runtime_available() -> bool:
+        return True
+
     def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen:
         cmd = [
             sys.executable,

dev_scripts/qa.py

@@ -108,9 +108,10 @@ Close the Dangerzone application and get the container image for that
 version. For example:
 
 ```
-$ docker images dangerzone.rocks/dangerzone:latest
+$ docker images dangerzone.rocks/dangerzone
 REPOSITORY                   TAG         IMAGE ID      CREATED       SIZE
 dangerzone.rocks/dangerzone  latest      <image ID>    <date>        <size>
+dangerzone.rocks/dangerzone  <tag>       <image ID>    <date>        <size>
 ```
 
 Then run the version under QA and ensure that the settings remain changed.
@@ -119,9 +120,10 @@ Afterwards check that new docker image was installed by running the same command
 and seeing the following differences:
 
 ```
-$ docker images dangerzone.rocks/dangerzone:latest
+$ docker images dangerzone.rocks/dangerzone
 REPOSITORY                   TAG         IMAGE ID        CREATED       SIZE
 dangerzone.rocks/dangerzone  latest      <different ID>  <newer date>  <different size>
+dangerzone.rocks/dangerzone  <other tag> <different ID>  <newer date>  <different size>
 ```
 
 #### 4. Dangerzone successfully installs the container image

install/common/build-image.py

@@ -2,12 +2,13 @@ import argparse
 import gzip
 import os
 import platform
+import secrets
 import subprocess
 import sys
 from pathlib import Path
 
 BUILD_CONTEXT = "dangerzone/"
-TAG = "dangerzone.rocks/dangerzone:latest"
+IMAGE_NAME = "dangerzone.rocks/dangerzone"
 REQUIREMENTS_TXT = "container-pip-requirements.txt"
 if platform.system() in ["Darwin", "Windows"]:
     CONTAINER_RUNTIME = "docker"
@@ -44,8 +45,31 @@ def main():
     )
     args = parser.parse_args()
 
+    tarball_path = Path("share") / "container.tar.gz"
+    image_id_path = Path("share") / "image-id.txt"
+
     print(f"Building for architecture '{ARCH}'")
 
+    # Designate a unique tag for this image, depending on the Git commit it was created
+    # from:
+    # 1. If created from a Git tag (e.g., 0.8.0), the image tag will be `0.8.0`.
+    # 2. If created from a commit, it will be something like `0.8.0-31-g6bdaa7a`.
+    # 3. If the contents of the Git repo are dirty, we will append a unique identifier
+    #    for this run, something like `0.8.0-31-g6bdaa7a-fdcb` or `0.8.0-fdcb`.
+    dirty_ident = secrets.token_hex(2)
+    tag = (
+        subprocess.check_output(
+            ["git", "describe", "--first-parent", f"--dirty=-{dirty_ident}"],
+        )
+        .decode()
+        .strip()[1:]  # remove the "v" prefix of the tag.
+    )
+    image_name_tagged = IMAGE_NAME + ":" + tag
+
+    print(f"Will tag the container image as '{image_name_tagged}'")
+    with open(image_id_path, "w") as f:
+        f.write(tag)
+
     print("Exporting container pip dependencies")
     with ContainerPipDependencies():
         if not args.use_cache:
@@ -59,8 +83,11 @@ def main():
                 check=True,
             )
 
+        # Build the container image, and tag it with two tags; the one we calculated
+        # above, and the "latest" tag.
         print("Building container image")
         cache_args = [] if args.use_cache else ["--no-cache"]
+        image_name_latest = IMAGE_NAME + ":latest"
         subprocess.run(
             [
                 args.runtime,
@@ -74,7 +101,9 @@ def main():
                 "-f",
                 "Dockerfile",
                 "--tag",
-                TAG,
+                image_name_latest,
+                "--tag",
+                image_name_tagged,
             ],
             check=True,
         )
@@ -85,7 +114,7 @@ def main():
                 [
                     CONTAINER_RUNTIME,
                     "save",
-                    TAG,
+                    image_name_tagged,
                 ],
                 stdout=subprocess.PIPE,
             )
@@ -93,7 +122,7 @@ def main():
             print("Compressing container image")
             chunk_size = 4 << 20
             with gzip.open(
-                "share/container.tar.gz",
+                tarball_path,
                 "wb",
                 compresslevel=args.compress_level,
             ) as gzip_f:
@@ -105,21 +134,6 @@ def main():
                         break
             cmd.wait(5)
 
-    print("Looking up the image id")
-    image_id = subprocess.check_output(
-        [
-            args.runtime,
-            "image",
-            "list",
-            "--format",
-            "{{.ID}}",
-            TAG,
-        ],
-        text=True,
-    )
-    with open("share/image-id.txt", "w") as f:
-        f.write(image_id)
-
 
 class ContainerPipDependencies:
     """Generates PIP dependencies within container"""

tests/gui/test_main_window.py

@@ -30,6 +30,7 @@ from dangerzone.isolation_provider.container import (
     NoContainerTechException,
     NotAvailableContainerTechException,
 )
+from dangerzone.isolation_provider.dummy import Dummy
 
 from .test_updater import assert_report_equal, default_updater_settings
 
@@ -510,9 +511,9 @@ def test_not_available_container_tech_exception(
 ) -> None:
     # Setup
     mock_app = mocker.MagicMock()
-    dummy = mocker.MagicMock()
+    dummy = Dummy()
-
+    fn = mocker.patch.object(dummy, "is_runtime_available")
-    dummy.is_runtime_available.side_effect = NotAvailableContainerTechException(
+    fn.side_effect = NotAvailableContainerTechException(
         "podman", "podman image ls logs"
     )
 

tests/isolation_provider/test_container.py

@@ -69,10 +69,11 @@ class TestContainer(IsolationProviderTest):
                 "image",
                 "list",
                 "--format",
-                "{{.ID}}",
+                "json",
                 "dangerzone.rocks/dangerzone",
             ],
             occurrences=2,
+            stdout="{}",
         )
 
         # Make podman load fail
@@ -102,10 +103,11 @@ class TestContainer(IsolationProviderTest):
                 "image",
                 "list",
                 "--format",
-                "{{.ID}}",
+                "json",
                 "dangerzone.rocks/dangerzone",
             ],
             occurrences=2,
+            stdout="{}",
         )
 
         # Patch gzip.open and podman load so that it works