[Containerapp] Secure Docker executable resolution

src/containerapp/HISTORY.rst

@@ -4,6 +4,7 @@ Release History
 ===============
 upcoming
 ++++++
+* Resolve Docker with Azure CLI's safe executable lookup before invoking it from local build and patch flows
 
 1.3.0b5
 ++++++

src/containerapp/azext_containerapp/_up_utils.py

@@ -21,6 +21,7 @@
     CLIError, CLIInternalError,
 )
 from azure.cli.core.commands.client_factory import get_subscription_id
+from azure.cli.core.util import get_safe_executable_path
 from azure.cli.command_modules.appservice._create_util import (
     check_resource_group_exists,
 )
@@ -534,7 +535,11 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False)
         from azure.cli.command_modules.acr.custom import acr_login
         from azure.cli.core.profiles import ResourceType
 
-        command = ['docker', 'push', image_name]
+        docker_exec_path = get_safe_executable_path("docker")
+        if not docker_exec_path:
+            raise CLIError("Docker executable could not be found.")
+
+        command = [docker_exec_path, 'push', image_name]
         logger.debug(f"Calling '{' '.join(command)}'")
         logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...")
         try:
@@ -547,14 +552,26 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False)
                         logger.warning(f"The current user isn't authenticated to the {self.acr.name} ACR instance. Triggering an ACR login and retrying to push the image...")
                         # Logic to login to ACR
                         task_command_kwargs = {"resource_type": ResourceType.MGMT_CONTAINERREGISTRY, 'operation_group': 'webhooks'}
+                        missing = object()
                         old_command_kwargs = {}
                         for key in task_command_kwargs:  # pylint: disable=consider-using-dict-items
-                            old_command_kwargs[key] = self.cmd.command_kwargs.get(key)
+                            old_command_kwargs[key] = self.cmd.command_kwargs.get(key, missing)
                             self.cmd.command_kwargs[key] = task_command_kwargs[key]
-                        if self.acr and self.acr.name is not None:
-                            acr_login(self.cmd, self.acr.name)
-                        for k, v in old_command_kwargs.items():
-                            self.cmd.command_kwargs[k] = v
+                        old_docker_command = os.environ.get("DOCKER_COMMAND")
+                        try:
+                            os.environ["DOCKER_COMMAND"] = docker_exec_path
+                            if self.acr and self.acr.name is not None:
+                                acr_login(self.cmd, self.acr.name)
+                        finally:
+                            if old_docker_command is None:
+                                os.environ.pop("DOCKER_COMMAND", None)
+                            else:
+                                os.environ["DOCKER_COMMAND"] = old_docker_command
+                            for k, v in old_command_kwargs.items():
+                                if v is missing:
+                                    self.cmd.command_kwargs.pop(k, None)
+                                else:
+                                    self.cmd.command_kwargs[k] = v
 
                         self._docker_push_to_container_registry(image_name, True)
                     else:

src/containerapp/azext_containerapp/_utils.py

@@ -13,7 +13,6 @@
 import hashlib
 import re
 import requests
-import shutil
 import packaging.version as SemVer
 import random
 
@@ -28,6 +27,7 @@
 from azure.cli.core._profile import Profile
 from azure.cli.core.azclierror import (ValidationError, ResourceNotFoundError, CLIError, InvalidArgumentValueError)
 from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id
+from azure.cli.core.util import get_safe_executable_path
 from azure.cli.command_modules.containerapp._utils import is_registry_msi_system
 from azure.mgmt.core.tools import parse_resource_id, is_valid_resource_id
 from ._utils_validation import validate_image_name  # noqa: F401
@@ -264,13 +264,17 @@ def list_environment_locations(cmd, resource_type=MANAGED_ENVIRONMENT_RESOURCE_T
 
 
 def is_docker_running():
+    docker_exec_path = get_safe_executable_path("docker")
+    if not docker_exec_path:
+        return False
+
     try:
         # Run a simple 'docker stats --no-stream' command to check if the Docker daemon is running
-        command = ["docker", "stats", "--no-stream"]
+        command = [docker_exec_path, "stats", "--no-stream"]
         with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process:
             _, _ = process.communicate()
             return process.returncode == 0
-    except Exception:
+    except (OSError, subprocess.SubprocessError):
         return False
 
 

src/containerapp/azext_containerapp/custom.py

@@ -25,6 +25,7 @@
     ResourceNotFoundError,
     ArgumentUsageError,
     MutuallyExclusiveArgumentError)
+from azure.cli.core.util import get_safe_executable_path
 from azure.cli.core.commands.client_factory import get_subscription_id
 from azure.cli.command_modules.containerapp.custom import set_secrets, open_containerapp_in_browser
 from azure.cli.command_modules.containerapp.containerapp_job_decorator import ContainerAppJobDecorator
@@ -1898,12 +1899,15 @@ def patch_cli_call(cmd, resource_group, container_app_name, container_name, targ
         logger.warning("Applying patch for container app: " + container_app_name + " container: " + container_name)
         validate_image_name(target_image_name)
         validate_image_name(new_run_image)
+        docker_exec_path = get_safe_executable_path("docker")
+        if not docker_exec_path:
+            raise CLIError("Docker executable could not be found.")
         subprocess.run([pack_exec_path, "rebase", "-q", target_image_name, "--run-image", new_run_image, "--force"], check=True)
         new_target_image_name = target_image_name.split(":")[0] + ":" + new_run_image.split(":")[1]
         validate_image_name(new_target_image_name)
-        subprocess.run(["docker", "tag", target_image_name, new_target_image_name], check=True)
+        subprocess.run([docker_exec_path, "tag", target_image_name, new_target_image_name], check=True)
         logger.debug(f"Publishing {new_target_image_name} to registry...")
-        subprocess.run(["docker", "push", "-q", new_target_image_name], check=True)
+        subprocess.run([docker_exec_path, "push", "-q", new_target_image_name], check=True)
         logger.warning("Patch applied and published successfully.\nNew image: " + new_target_image_name)
     except Exception:
         logger.error("Error: Failed to apply patch and publish. Check if registry is logged in and has write access.")

src/containerapp/azext_containerapp/tests/latest/test_containerapp_patch_unit.py

@@ -8,7 +8,7 @@
 import unittest
 from unittest import mock
 
-from azure.cli.core.azclierror import ValidationError
+from azure.cli.core.azclierror import CLIError, ValidationError
 
 from azext_containerapp._utils_validation import validate_image_name
 
@@ -82,7 +82,9 @@ def test_rejects_newline(self):
     "subprocess": __import__("subprocess"),
     "json": __import__("json"),
     "logger": mock.MagicMock(),
+    "CLIError": CLIError,
     "validate_image_name": validate_image_name,
+    "get_safe_executable_path": lambda _: "/usr/local/bin/docker",
     "update_containerapp": mock.MagicMock(return_value={}),
 }
 
@@ -207,16 +209,36 @@ def test_calls_subprocess_run_with_list_args(self):
         tag_call = mock_subprocess.run.call_args_list[1]
         self.assertEqual(
             tag_call[0][0],
-            ["docker", "tag", "myregistry.azurecr.io/myapp:old-tag",
+            ["/usr/local/bin/docker", "tag", "myregistry.azurecr.io/myapp:old-tag",
              "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"]
         )
 
         push_call = mock_subprocess.run.call_args_list[2]
         self.assertEqual(
             push_call[0][0],
-            ["docker", "push", "-q", "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"]
+            ["/usr/local/bin/docker", "push", "-q", "myregistry.azurecr.io/myapp:7.1.0-debian-bullseye"]
         )
 
+    def test_does_not_run_when_docker_is_not_found(self):
+        mock_subprocess = mock.MagicMock()
+
+        with mock.patch.dict(
+                _FUNC_GLOBALS,
+                {"subprocess": mock_subprocess, "get_safe_executable_path": lambda _: None}
+        ):
+            fn = _extract_function("patch_cli_call")
+            with self.assertRaises(CLIError):
+                fn(
+                    cmd=mock.MagicMock(),
+                    resource_group="myrg",
+                    container_app_name="myapp",
+                    container_name="mycontainer",
+                    target_image_name="myregistry.azurecr.io/myapp:old-tag",
+                    new_run_image="mcr.microsoft.com/oryx/dotnetcore:7.1.0-debian-bullseye",
+                    pack_exec_path="/usr/local/bin/pack",
+                )
+            mock_subprocess.run.assert_not_called()
+
     def test_rejects_malicious_target_image_name(self):
         mock_subprocess = mock.MagicMock()
 

src/containerapp/azext_containerapp/tests/latest/test_utils.py

@@ -3,9 +3,173 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
+import os
 import unittest
-from azure.cli.core.azclierror import ValidationError
-from azext_containerapp._utils import validate_environment_mode_and_workload_profiles_compatible
+from unittest import mock
+
+from azure.cli.core.azclierror import CLIError, ValidationError
+from azext_containerapp._utils import (
+    is_docker_running,
+    validate_environment_mode_and_workload_profiles_compatible,
+)
+
+
+def _extract_up_utils_method(class_name, method_name):
+    import ast
+    import textwrap
+
+    src_path = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "_up_utils.py"))
+    with open(src_path, "r", encoding="utf-8") as file:
+        source = file.read()
+    tree = ast.parse(source, filename=src_path)
+
+    for node in tree.body:
+        if isinstance(node, ast.ClassDef) and node.name == class_name:
+            for child in node.body:
+                if isinstance(child, ast.FunctionDef) and child.name == method_name:
+                    namespace = {
+                        "CLIError": CLIError,
+                        "get_safe_executable_path": mock.MagicMock(),
+                        "logger": mock.MagicMock(),
+                        "os": os,
+                        "subprocess": __import__("subprocess"),
+                    }
+                    method_source = ast.get_source_segment(source, child)
+                    exec(compile(textwrap.dedent(method_source), src_path, "exec"), namespace)  # pylint: disable=exec-used
+                    return namespace[method_name]
+
+    raise ValueError(f"Method {class_name}.{method_name} not found in {src_path}")
+
+
+class TestDockerExecutablePath(unittest.TestCase):
+    def test_is_docker_running_uses_resolved_path(self):
+        docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe")
+        process = mock.MagicMock()
+        process.returncode = 0
+        process.communicate.return_value = (b"", b"")
+        process.__enter__.return_value = process
+
+        with mock.patch("azext_containerapp._utils.get_safe_executable_path", return_value=docker_path) as resolve, \
+                mock.patch("azext_containerapp._utils.subprocess.Popen", return_value=process) as popen:
+            self.assertTrue(is_docker_running())
+
+        resolve.assert_called_once_with("docker")
+        popen.assert_called_once_with(
+            [docker_path, "stats", "--no-stream"],
+            stdout=mock.ANY,
+            stderr=mock.ANY,
+        )
+
+    def test_is_docker_running_returns_false_when_docker_is_not_found(self):
+        with mock.patch("azext_containerapp._utils.get_safe_executable_path", return_value=None) as resolve, \
+                mock.patch("azext_containerapp._utils.subprocess.Popen") as popen:
+            self.assertFalse(is_docker_running())
+
+        resolve.assert_called_once_with("docker")
+        popen.assert_not_called()
+
+
+class TestDockerPushPath(unittest.TestCase):
+    @staticmethod
+    def _create_app(method):
+        app_type = type("TestContainerApp", (), {"_docker_push_to_container_registry": method})
+        app = app_type()
+        app.acr = mock.MagicMock()
+        app.acr.name = "testregistry"
+        app.cmd = mock.MagicMock()
+        app.cmd.command_kwargs = {
+            "resource_type": "old-resource-type",
+            "operation_group": "old-operation-group",
+        }
+        return app
+
+    @staticmethod
+    def _create_process(returncode, stderr=b""):
+        process = mock.MagicMock()
+        process.returncode = returncode
+        process.communicate.return_value = (b"", stderr)
+        process.__enter__.return_value = process
+        return process
+
+    def test_push_uses_resolved_docker_path(self):
+        method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry")
+        app = self._create_app(method)
+        docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe")
+        mock_subprocess = mock.MagicMock()
+        mock_subprocess.PIPE = __import__("subprocess").PIPE
+        mock_subprocess.Popen.return_value = self._create_process(0)
+
+        with mock.patch.dict(
+                method.__globals__,
+                {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess}
+        ):
+            app._docker_push_to_container_registry("registry/image:tag")
+
+        mock_subprocess.Popen.assert_called_once_with(
+            [docker_path, "push", "registry/image:tag"],
+            stdout=mock_subprocess.PIPE,
+            stderr=mock_subprocess.PIPE,
+        )
+
+    def test_acr_login_uses_resolved_path_and_restores_environment(self):
+        method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry")
+        app = self._create_app(method)
+        docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe")
+        mock_subprocess = mock.MagicMock()
+        mock_subprocess.PIPE = __import__("subprocess").PIPE
+        mock_subprocess.Popen.side_effect = [
+            self._create_process(1, b"unauthorized"),
+            self._create_process(0),
+        ]
+
+        def assert_safe_docker_command(*_):
+            self.assertEqual(os.environ["DOCKER_COMMAND"], docker_path)
+
+        original_command_kwargs = dict(app.cmd.command_kwargs)
+        with mock.patch.dict(
+                method.__globals__,
+                {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess}
+        ), mock.patch(
+                "azure.cli.command_modules.acr.custom.acr_login",
+                side_effect=assert_safe_docker_command
+        ) as acr_login, mock.patch.dict(
+                os.environ,
+                {"DOCKER_COMMAND": "original-docker-command"}
+        ):
+            app._docker_push_to_container_registry("testregistry.azurecr.io/image:tag")
+            self.assertEqual(os.environ["DOCKER_COMMAND"], "original-docker-command")
+
+        acr_login.assert_called_once_with(app.cmd, "testregistry")
+        self.assertEqual(app.cmd.command_kwargs, original_command_kwargs)
+        self.assertEqual(mock_subprocess.Popen.call_count, 2)
+        for call in mock_subprocess.Popen.call_args_list:
+            self.assertEqual(call.args[0][0], docker_path)
+
+    def test_acr_login_restores_absent_command_kwargs(self):
+        method = _extract_up_utils_method("ContainerApp", "_docker_push_to_container_registry")
+        app = self._create_app(method)
+        app.cmd.command_kwargs = {}
+        docker_path = os.path.join(os.path.abspath(os.sep), "trusted", "docker.exe")
+        mock_subprocess = mock.MagicMock()
+        mock_subprocess.PIPE = __import__("subprocess").PIPE
+        mock_subprocess.Popen.return_value = self._create_process(1, b"unauthorized")
+
+        with mock.patch.dict(
+                method.__globals__,
+                {"get_safe_executable_path": lambda _: docker_path, "subprocess": mock_subprocess}
+        ), mock.patch(
+                "azure.cli.command_modules.acr.custom.acr_login",
+                side_effect=CLIError("login failed")
+        ), mock.patch.dict(
+                os.environ,
+                {},
+                clear=True
+        ):
+            with self.assertRaises(CLIError):
+                app._docker_push_to_container_registry("testregistry.azurecr.io/image:tag")
+
+        self.assertEqual(app.cmd.command_kwargs, {})
+        self.assertNotIn("DOCKER_COMMAND", os.environ)
 
 
 class TestResolveEnvironmentModeAndWorkloadProfiles(unittest.TestCase):