Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a temporary "scratch" volume to plugins #363

Merged
merged 13 commits into from
Nov 20, 2024
Merged
1 change: 1 addition & 0 deletions backend/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 10 additions & 1 deletion backend/Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# }
# ]

log = Logger("docker_builder")
log = Logger("oci_builder")


class ImageBuilder:
Expand Down Expand Up @@ -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"]):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from artemislib.logging import Logger

logger = Logger("docker_remover")
logger = Logger("oci_remover")


def remove_docker_image(image: dict):
Expand Down
6 changes: 3 additions & 3 deletions backend/engine/tests/test_docker_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__))

Expand All @@ -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):
Expand All @@ -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
Expand Down
33 changes: 32 additions & 1 deletion backend/engine/tests/test_engine_utils.py
Original file line number Diff line number Diff line change
@@ -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__))
Expand Down Expand Up @@ -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)
2 changes: 1 addition & 1 deletion backend/engine/tests/test_trivy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion backend/engine/tests/test_trivy_sbom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion backend/engine/tests/test_trivy_sca.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions backend/engine/utils/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
103 changes: 81 additions & 22 deletions backend/engine/utils/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand All @@ -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"],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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),
]
Expand Down
Loading