fix(tests): update error handling for host path creation in Docker volume validation

Author: RonaldJEN

sdks/sandbox/python/src/opensandbox/adapters/filesystem_adapter.py

@@ -25,6 +25,7 @@
 from collections.abc import AsyncIterator
 from io import IOBase, TextIOBase
 from typing import TypedDict
+from urllib.parse import quote
 
 import httpx
 
@@ -52,7 +53,7 @@
 
 class _DownloadRequest(TypedDict):
     url: str
-    params: dict[str, str]
+    params: dict[str, str] | None
     headers: dict[str, str]
 
 
@@ -158,11 +159,13 @@ async def read_bytes(
             request_data = self._build_download_request(path, range_header)
             client = await self._get_httpx_client()
 
-            response = await client.get(
-                request_data["url"],
-                params=request_data["params"],
-                headers=request_data["headers"],
-            )
+            request_kwargs: dict[str, dict[str, str]] = {
+                "headers": request_data["headers"],
+            }
+            if request_data["params"] is not None:
+                request_kwargs["params"] = request_data["params"]
+
+            response = await client.get(request_data["url"], **request_kwargs)
             response.raise_for_status()
             return response.content
         except Exception as e:
@@ -186,12 +189,13 @@ async def read_bytes_stream(
             params = request_data["params"]
             headers = request_data["headers"]
 
-            request = client.build_request(
-                "GET",
-                url,
-                params=params,
-                headers=headers,
-            )
+            request_kwargs: dict[str, dict[str, str]] = {
+                "headers": headers,
+            }
+            if params is not None:
+                request_kwargs["params"] = params
+
+            request = client.build_request("GET", url, **request_kwargs)
 
             response = await client.send(request, stream=True)
 
@@ -485,6 +489,6 @@ def _build_download_request(
 
         return {
             "url": url,
-            "params": {},
+            "params": None,
             "headers": headers,
         }

sdks/sandbox/python/src/opensandbox/sync/adapters/filesystem_adapter.py

@@ -22,6 +22,7 @@
 from collections.abc import Iterator
 from io import IOBase, TextIOBase
 from typing import TypedDict
+from urllib.parse import quote
 
 import httpx
 
@@ -49,7 +50,7 @@
 
 class _DownloadRequest(TypedDict):
     url: str
-    params: dict[str, str]
+    params: dict[str, str] | None
     headers: dict[str, str]
 
 
@@ -92,7 +93,7 @@ def _build_download_request(self, path: str, range_header: str | None = None) ->
         headers: dict[str, str] = {}
         if range_header:
             headers["Range"] = range_header
-        return {"url": url, "params": {}, "headers": headers}
+        return {"url": url, "params": None, "headers": headers}
 
     def read_file(
         self,
@@ -108,11 +109,13 @@ def read_bytes(self, path: str, *, range_header: str | None = None) -> bytes:
         logger.debug("Reading file as bytes: %s", path)
         try:
             request_data = self._build_download_request(path, range_header)
-            response = self._httpx_client.get(
-                request_data["url"],
-                params=request_data["params"],
-                headers=request_data["headers"],
-            )
+            request_kwargs: dict[str, dict[str, str]] = {
+                "headers": request_data["headers"],
+            }
+            if request_data["params"] is not None:
+                request_kwargs["params"] = request_data["params"]
+
+            response = self._httpx_client.get(request_data["url"], **request_kwargs)
             response.raise_for_status()
             return response.content
         except Exception as e:
@@ -128,7 +131,11 @@ def read_bytes_stream(
         params = request_data["params"]
         headers = request_data["headers"]
 
-        request = self._httpx_client.build_request("GET", url, params=params, headers=headers)
+        request_kwargs: dict[str, dict[str, str]] = {"headers": headers}
+        if params is not None:
+            request_kwargs["params"] = params
+
+        request = self._httpx_client.build_request("GET", url, **request_kwargs)
         response = self._httpx_client.send(request, stream=True)
 
         if response.status_code >= 300:

server/tests/test_docker_service.py

@@ -988,7 +988,7 @@ def test_pvc_subpath_binds_resolved_to_mountpoint(self, mock_docker):
         assert binds[0] == "/var/lib/docker/volumes/my-vol/_data/datasets/train:/mnt/train:ro"
 
     def test_host_path_not_found_rejected(self, mock_docker):
-        """Host path that does not exist should be rejected."""
+        """Host path create failure should return 500 with HOST_PATH_CREATE_FAILED."""
         mock_client = MagicMock()
         mock_client.containers.list.return_value = []
         mock_docker.from_env.return_value = mock_client
@@ -1012,11 +1012,12 @@ def test_host_path_not_found_rejected(self, mock_docker):
             ],
         )
 
-        with pytest.raises(HTTPException) as exc_info:
-            service.create_sandbox(request)
+        with patch("src.services.docker.os.makedirs", side_effect=PermissionError("denied")):
+            with pytest.raises(HTTPException) as exc_info:
+                service.create_sandbox(request)
 
-        assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
-        assert exc_info.value.detail["code"] == SandboxErrorCodes.HOST_PATH_NOT_FOUND
+        assert exc_info.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
+        assert exc_info.value.detail["code"] == SandboxErrorCodes.HOST_PATH_CREATE_FAILED
 
     def test_host_path_not_in_allowlist_rejected(self, mock_docker):
         """Host path not in allowlist should be rejected."""
@@ -1167,17 +1168,21 @@ def test_host_volume_with_subpath_resolved_correctly(self, mock_docker):
             assert len(binds) == 1
             assert binds[0] == f"{sub_dir}:/mnt/work:ro"
 
-    def test_host_subpath_not_found_rejected(self, mock_docker):
-        """Host volume with non-existent subPath should be rejected."""
+    def test_host_subpath_auto_created(self, mock_docker):
+        """Host volume with non-existent subPath should be auto-created."""
         mock_client = MagicMock()
         mock_client.containers.list.return_value = []
+        mock_client.api.create_host_config.return_value = {}
+        mock_client.api.create_container.return_value = {"Id": "cid"}
+        mock_client.containers.get.return_value = MagicMock()
         mock_docker.from_env.return_value = mock_client
 
         service = DockerSandboxService(config=_app_config())
 
         import tempfile
 
         with tempfile.TemporaryDirectory() as tmpdir:
+            sub = "auto-created-sub"
             request = CreateSandboxRequest(
                 image=ImageSpec(uri="python:3.11"),
                 timeout=120,
@@ -1191,16 +1196,31 @@ def test_host_subpath_not_found_rejected(self, mock_docker):
                         host=Host(path=tmpdir),
                         mount_path="/mnt/work",
                         read_only=False,
-                        sub_path="nonexistent-sub",
+                        sub_path=sub,
                     )
                 ],
             )
 
-            with pytest.raises(HTTPException) as exc_info:
-                service.create_sandbox(request)
+            import os
+
+            resolved = os.path.join(tmpdir, sub)
+            assert not os.path.exists(resolved)
 
-            assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
-            assert exc_info.value.detail["code"] == SandboxErrorCodes.HOST_PATH_NOT_FOUND
+            # create_sandbox will proceed past volume validation (subpath
+            # auto-created) but will fail later during container provisioning
+            # (mock doesn't cover the full flow).  We only care that the
+            # directory was created — NOT that it raised HOST_PATH_CREATE_FAILED.
+            try:
+                service.create_sandbox(request)
+            except HTTPException as e:
+                # If it's our own create-failed error, the auto-create didn't
+                # work — let the test fail explicitly.
+                if e.detail.get("code") == SandboxErrorCodes.HOST_PATH_CREATE_FAILED:
+                    raise
+            except Exception:
+                pass  # other provisioning errors are expected
+
+            assert os.path.isdir(resolved)
 
     def test_empty_allowlist_permits_any_host_path(self, mock_docker):
         """Empty allowed_host_paths (default) should permit any valid host path."""