Provide a temporary "scratch" volume to plugins (#363)

* Rename "docker" to "oci".

Side-step name conflict with Docker SDK for Python.

* Add Docker SDK.

* Fix crash on invalid JSON in secret.

* Provide a temp named volume to each plugin.

The temporary volume allows containers-within-containers to share a
writable "scratch space" which is deleted after the plugin completes.
Otherwise, if a container launches another container, it would need to
know what is safe to bind mount from the host (since bind mounts are
always mounted from the host system).

* Mount target dir with same path in container.

This more closely matches the way the deployed engine container
mounts the "/work" directory from the host.

This is important for plugin containers that want to mount the source
directory into a sub-container.

* Provide a temporary volume to the plugin.

* Move temp vol mount and pass mount point.

Moved the temporary named volume mount point from /work/tmp to
/tmp/work. If the container mounts /work as read-only, then Docker can't
mkdir /work/tmp.  /tmp is a safer choice since it should always be
writable.

We also now pass the mount point in the temp_vol_name engine var so that
if we change it again later, plugins will be able to adapt.

* Use plugin ID instead of name in vol name.

* Mount target dir in engine and inherit.

We now mount the target dir in the engine and inherit the volume in the
plugin container. This is much closer to the deployed engine and denies
the assumption that the host path matches the mount path.

* Revert target path examples.

The mount dir is no longer matched to the host dir.

* Pass the engine ID.

backend/Pipfile

@@ -15,6 +15,7 @@ boto3-stubs = {extras = ["ec2", "lambda", "s3", "secretsmanager", "sqs"], versio
 cryptography = "~=43.0"
 cwe2 = "~=3.0"
 django = "~=3.2"
+docker = "~=7.1"
 graphql-query = "~=1.4"
 joserfc = "~=1.0"
 packaging = "==23.2"  # Must match version in Makefile.

backend/engine/docker/builder.py → backend/engine/oci/builder.py

@@ -19,7 +19,7 @@
 #   }
 # ]
 
-log = Logger("docker_builder")
+log = Logger("oci_builder")
 
 
 class ImageBuilder:
@@ -156,6 +156,10 @@ def private_docker_repos_login(self, files) -> None:
         else:
             return
 
+        if not private_docker_repos_response:
+            # Error already logged in convert_string_to_json.
+            return
+
         for repo in private_docker_repos_response:
             log.info("Checking if any Dockerfiles depend on %s", repo["url"])
             if self.docker_login_needed(files, repo["search"], repo["url"]):

backend/engine/docker/remover.py → backend/engine/oci/remover.py

@@ -2,7 +2,7 @@
 
 from artemislib.logging import Logger
 
-logger = Logger("docker_remover")
+logger = Logger("oci_remover")
 
 
 def remove_docker_image(image: dict):

backend/engine/tests/test_docker_builder.py

@@ -6,7 +6,7 @@
 import pytest
 
 from artemislib.logging import Logger
-from docker import builder
+from oci import builder
 
 TEST_DIR = os.path.dirname(os.path.abspath(__file__))
 
@@ -24,7 +24,7 @@
 
 TEST_GET_SECRET_WITH_STATUS_MOCK_OUTPUT = {"status": True, "response": json.dumps(TEST_PRIVATE_DOCKER_REPOS_CONFIGS)}
 
-TEST_LOGGER = Logger("docker_builder")
+TEST_LOGGER = Logger("oci_builder")
 
 
 class TestDockerUtil(unittest.TestCase):
@@ -40,7 +40,7 @@ def test_find_dockerfiles(self):
     def test_private_docker_repos_login(self):
         image_builder = builder.ImageBuilder(os.path.join(TEST_ROOT, "Dockerfiles"), None, None, None)
         with patch("plugins.lib.utils.get_secret_with_status") as mock_get_secret_with_status:
-            with patch("docker.builder.ImageBuilder.docker_login_needed") as mock_docker_login_needed:
+            with patch("oci.builder.ImageBuilder.docker_login_needed") as mock_docker_login_needed:
                 mock_get_secret_with_status.return_value = TEST_GET_SECRET_WITH_STATUS_MOCK_OUTPUT
 
                 # return true to test docker_login arguments

backend/engine/tests/test_engine_utils.py

@@ -1,10 +1,18 @@
+import docker
 import os
 from typing import Any
 import unittest
 from unittest.mock import patch
+import docker.errors
 from pydantic import ValidationError
 
-from engine.utils.plugin import PluginSettings, Runner, get_plugin_settings, match_nonallowlisted_raw_secrets
+from engine.utils.plugin import (
+    PluginSettings,
+    Runner,
+    get_plugin_settings,
+    match_nonallowlisted_raw_secrets,
+    temporary_volume,
+)
 from utils.services import _get_services_from_file
 
 TEST_DIR = os.path.dirname(os.path.abspath(__file__))
@@ -151,3 +159,26 @@ def test_get_plugin_settings_invalid(self):
         with self.assertRaises(ValidationError) as ex:
             get_plugin_settings("invalid")
         self.assertEqual(ex.exception.error_count(), 4)
+
+    def test_temporary_volume_normal(self):
+        """
+        Tests a temporary volume is created and automatically cleaned up.
+        """
+        docker_client = docker.from_env()
+        with temporary_volume("test-prefix") as vol_name:
+            self.assertTrue(vol_name.startswith("test-prefix"))
+            docker_client.volumes.get(vol_name)
+        with self.assertRaises(docker.errors.NotFound):
+            docker_client.volumes.get(vol_name)
+
+    def test_temporary_volume_cleanup(self):
+        """
+        Tests a temporary volume is cleaned up even if an exception is raised.
+        """
+        docker_client = docker.from_env()
+        vol_name = ""
+        with self.assertRaises(ValueError):
+            with temporary_volume("test-prefix") as vol_name:
+                raise ValueError("test")
+        with self.assertRaises(docker.errors.NotFound):
+            docker_client.volumes.get(vol_name)

backend/engine/tests/test_trivy.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from docker import builder, remover
+from oci import builder, remover
 from engine.plugins.trivy import main as Trivy
 from engine.plugins.lib.utils import convert_string_to_json
 from engine.plugins.lib.utils import setup_logging

backend/engine/tests/test_trivy_sbom.py

@@ -3,7 +3,7 @@
 import unittest
 import pytest
 import secrets
-from docker import builder, remover
+from oci import builder, remover
 from subprocess import CompletedProcess
 from unittest.mock import patch
 from engine.plugins.trivy_sbom import main as Trivy

backend/engine/tests/test_trivy_sca.py

@@ -5,7 +5,7 @@
 
 from subprocess import CompletedProcess
 from unittest.mock import patch
-from docker import remover
+from oci import remover
 from engine.plugins.trivy_sca import main as Trivy
 from engine.plugins.lib.trivy_common.generate_locks import check_package_files
 from engine.plugins.lib.utils import convert_string_to_json

backend/engine/utils/engine.py

@@ -4,8 +4,8 @@
 from botocore.exceptions import ClientError
 
 from artemislib.logging import Logger
-from docker import remover
-from docker.builder import ImageBuilder
+from oci import remover
+from oci.builder import ImageBuilder
 from env import APPLICATION, ECR, REGION
 
 DYNAMODB_TTL_DAYS = 60

backend/engine/utils/plugin.py

@@ -2,16 +2,20 @@
 from enum import Enum
 import os
 import subprocess
+from contextlib import contextmanager
 from dataclasses import dataclass
 from datetime import datetime, timezone
 from fnmatch import fnmatch
 from typing import Optional, Union
 from urllib.parse import quote_plus
+import uuid
 
 import boto3
 from botocore.exceptions import ClientError
 from django.db.models import Q
 from django.db import transaction
+import docker
+import docker.errors
 from pydantic import BaseModel, Field, field_validator
 
 from artemisdb.artemisdb.models import PluginConfig, SecretType, PluginType, Scan
@@ -42,6 +46,12 @@
 
 UI_SECRETS_TAB_INDEX = 3
 
+TEMP_VOLUME_NAME_PREFIX = "artemis-plugin-temp-"
+TEMP_VOLUME_LABEL = "artemis.temp"
+TEMP_VOLUME_MOUNT = "/tmp/work"
+
+docker_client = docker.from_env()
+
 
 class Runner(str, Enum):
     """
@@ -132,7 +142,7 @@ def _parse_disabled(cls, enabled: Union[str, bool]) -> bool:
         return True
 
 
-def get_engine_vars(scan: Scan, depth: Optional[str] = None, include_dev=False, services=None):
+def get_engine_vars(scan: Scan, temp_vol_name: str, depth: Optional[str] = None, include_dev=False, services=None):
     """
     Returns a json str that can be converted back to a dict by the plugin.
     The object will container information known to the engine
@@ -151,6 +161,7 @@ def get_engine_vars(scan: Scan, depth: Optional[str] = None, include_dev=False,
             "depth": depth,
             "include_dev": include_dev,
             "engine_id": ENGINE_ID,
+            "temp_vol_name": f"{temp_vol_name}:{TEMP_VOLUME_MOUNT}",
             "java_heap_size": PLUGIN_JAVA_HEAP_SIZE,
             "service_name": scan.repo.service,
             "service_type": services[scan.repo.service]["type"],
@@ -261,6 +272,31 @@ def _get_plugin_config(plugin: str, full_repo: str) -> dict:
     return {}
 
 
+@contextmanager
+def temporary_volume(name_prefix: str):
+    """
+    Creates a temporary volume for a plugin.
+    The generated name of the volume is passed to the block.
+    The volume is removed automatically.
+    """
+    name = f"{name_prefix}-{uuid.uuid4()}"
+
+    # If the volume creation fails, we pass through the exception.
+    # The label is set in order to be able to later detect volumes that
+    # failed to be cleaned up.
+    log.info(f"Creating temporary volume: {name}")
+    vol = docker_client.volumes.create(name, labels={TEMP_VOLUME_LABEL: "1"})
+
+    try:
+        yield name
+    finally:
+        try:
+            log.info(f"Removing temporary volume: {name}")
+            vol.remove(True)
+        except docker.errors.APIError as ex:
+            log.error(f"Failed to remove volume: {name}", exc_info=ex)
+
+
 def run_plugin(
     plugin: str,
     scan: Scan,
@@ -321,27 +357,44 @@ def run_plugin(
     full_repo = f"{scan.repo.service}/{scan.repo.repo}"
     plugin_config = _get_plugin_config(plugin, full_repo)
 
-    plugin_command = get_plugin_command(
-        scan, plugin, settings, depth, include_dev, scan_images, plugin_config, services
-    )
-
-    try:
-        # Run the plugin inside the settings.image
-        r = subprocess.run(
-            plugin_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False, timeout=settings.timeout
-        )
-    except subprocess.TimeoutExpired:
-        return Result(
-            name=settings.name,
-            type=settings.plugin_type,
-            success=False,
-            truncated=False,
-            details=[],
-            errors=[f"Plugin {settings.name} exceeded maximum runtime ({settings.timeout} seconds)."],
-            alerts=[],
-            debug=[],
-            dirty=settings.writable,
+    # The plugin container may want to launch another container via
+    # "docker run" (i.e. the plugin is running a tool provided as a container
+    # image).
+    #
+    # We provide a temporary named volume for the plugin to share (by name),
+    # with any containers it launches. This side-steps two issues:
+    # - Plugin containers can't bind mount from their own filesystem (bind
+    #   mounts are always sourced from the host).
+    # - Plugin containers can't create a volume themselves and mount it into
+    #   their own filesystem.
+    #
+    # The temporary named volume is automatically deleted after the plugin
+    # container exits.
+    with temporary_volume(f"{TEMP_VOLUME_NAME_PREFIX}-{plugin}") as volname:
+        plugin_command = get_plugin_command(
+            scan, plugin, settings, depth, include_dev, volname, scan_images, plugin_config, services
         )
+        try:
+            # Run the plugin inside the settings.image
+            r = subprocess.run(
+                plugin_command,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                check=False,
+                timeout=settings.timeout,
+            )
+        except subprocess.TimeoutExpired:
+            return Result(
+                name=settings.name,
+                type=settings.plugin_type,
+                success=False,
+                truncated=False,
+                details=[],
+                errors=[f"Plugin {settings.name} exceeded maximum runtime ({settings.timeout} seconds)."],
+                alerts=[],
+                debug=[],
+                dirty=settings.writable,
+            )
 
     inject_plugin_logs(r.stderr.decode("utf-8"), plugin)
 
@@ -625,6 +678,7 @@ def get_plugin_command(
     settings: PluginSettings,
     depth: Optional[str],
     include_dev: bool,
+    temp_vol_name: str,
     scan_images,
     plugin_config,
     services,
@@ -647,6 +701,11 @@ def get_plugin_command(
         working_mount,
     ]
 
+    # The named temporary volume allows a plugin container to share the
+    # volume with other containers without needing to know anything
+    # about bind-mounted volumes from the host.
+    cmd.extend(["-v", f"{temp_vol_name}:{TEMP_VOLUME_MOUNT}:nocopy"])
+
     if profile:
         # When running locally AWS_PROFILE may be set. If so, pass the credentials and profile name down to the plugin
         # container. Also pass down the local DB connection info.
@@ -719,7 +778,7 @@ def get_plugin_command(
     # Arguments passed to the plugin.
     cmd.extend(
         [
-            get_engine_vars(scan, depth=depth, include_dev=include_dev, services=services),
+            get_engine_vars(scan, temp_vol_name, depth=depth, include_dev=include_dev, services=services),
             json.dumps(scan_images),
             json.dumps(plugin_config),
         ]