From f11be0a7bcadc0f35234296425d2e5adfc761bf9 Mon Sep 17 00:00:00 2001 From: siye566 <114060217+siye566@users.noreply.github.com> Date: Sat, 3 Oct 2026 08:27:54 +0800 Subject: [PATCH] fix(sandbox): pin utf-8 decoding on the local sandbox backend's subprocess calls (#5905) * fix(sandbox): pin utf-8 decoding on the local sandbox backend's subprocess calls The local container backend shells out to docker / the container runtime with text-mode subprocess calls that do not pass an encoding, so output is decoded with the platform's preferred encoding. On hosts whose ANSI code page is not UTF-8 (cp936 Chinese Windows, cp1252 Western European), any non-ASCII byte in the CLI output raises inside subprocess's reader thread: stdout/stderr come back None, the JSON-parsing call sites then crash on None or silently mis-parse, and stderr diagnostics are dropped. Pin encoding="utf-8" with errors="replace" on every text-mode call in the module. The container CLIs write UTF-8 on every platform; replace keeps the parseable portion of any non-conforming output alive instead of discarding it wholesale. * test(sandbox): validate UTF-8 subprocess settings per call --------- Co-authored-by: Willem Jiang --- .../community/aio_sandbox/local_backend.py | 65 ++++++++++++++++++- backend/tests/AGENTS.md | 4 ++ .../tests/test_aio_sandbox_local_backend.py | 57 ++++++++++++++++ 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py index 14b4bd4c65..a3c59008bb 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py @@ -259,6 +259,8 @@ def _docker_bridge_gateway_ip() -> str | None: ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) except (OSError, subprocess.TimeoutExpired) as e: @@ -289,6 +291,8 @@ def _docker_server_is_desktop() -> bool: ["docker", "info", "--format", "{{json .OperatingSystem}}"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as exc: @@ -723,6 +727,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "network", "inspect", network_name], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as exc: @@ -793,6 +799,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "version", "--format", "{{.Server.Version}}"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", check=True, timeout=10, ) @@ -829,6 +837,8 @@ class LocalContainerBackend(SandboxBackend): ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=5, ) except (FileNotFoundError, subprocess.TimeoutExpired, OSError): @@ -853,6 +863,8 @@ class LocalContainerBackend(SandboxBackend): ["container", "--version"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", check=True, timeout=5, ) @@ -1065,6 +1077,8 @@ class LocalContainerBackend(SandboxBackend): ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) if result.returncode != 0: @@ -1091,6 +1105,8 @@ class LocalContainerBackend(SandboxBackend): ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) if result.returncode != 0: @@ -1154,18 +1170,34 @@ class LocalContainerBackend(SandboxBackend): ] # First use may pull the sidecar image. Match the sandbox create path's # tolerance for an image download instead of killing Docker mid-pull. - created = subprocess.run(cmd, capture_output=True, text=True, timeout=300) + created = subprocess.run( + cmd, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=300, + ) if created.returncode != 0: raise RuntimeError(f"Failed to create sandbox network proxy: {created.stderr.strip()}") connected = subprocess.run( ["docker", "network", "connect", network_name, proxy_name], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) if connected.returncode != 0: raise RuntimeError(f"Failed to connect sandbox network proxy: {connected.stderr.strip()}") - started = subprocess.run(["docker", "start", proxy_name], capture_output=True, text=True, timeout=15) + started = subprocess.run( + ["docker", "start", proxy_name], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=15, + ) if started.returncode != 0: raise RuntimeError(f"Failed to start sandbox network proxy: {started.stderr.strip()}") source = Path(__file__).with_name("network_proxy.py") @@ -1373,6 +1405,8 @@ class LocalContainerBackend(SandboxBackend): ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) if result.returncode != 0: @@ -1492,6 +1526,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "rm", "-f", proxy_name], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) if removed.returncode != 0 and "no such container" not in (removed.stderr or "").lower(): @@ -1501,6 +1537,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "network", "rm", current_network_name], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) if result.returncode != 0 and "not found" not in (result.stderr or "").lower(): @@ -1514,6 +1552,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "exec", proxy_name, "python", _NETWORK_PROXY_CONTAINER_SCRIPT, "pending"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) if result.returncode != 0: @@ -1535,6 +1575,8 @@ class LocalContainerBackend(SandboxBackend): ["docker", "exec", proxy_name, "python", _NETWORK_PROXY_CONTAINER_SCRIPT, "deny-pending"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) if result.returncode != 0: @@ -1562,6 +1604,8 @@ class LocalContainerBackend(SandboxBackend): ], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=10, ) return result.returncode == 0 @@ -1579,6 +1623,8 @@ class LocalContainerBackend(SandboxBackend): [self._runtime, "inspect", *container_names], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=15, ) except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, OSError) as e: @@ -1873,7 +1919,14 @@ class LocalContainerBackend(SandboxBackend): logger.info(f"Starting container using {self._runtime}: {log_cmd}") try: - result = subprocess.run(cmd, capture_output=True, text=True, check=True) + result = subprocess.run( + cmd, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=True, + ) container_id = result.stdout.strip() logger.info(f"Started container {container_name} (ID: {container_id}) using {self._runtime}") return container_id @@ -1897,6 +1950,8 @@ class LocalContainerBackend(SandboxBackend): [self._runtime, "stop", container_id], capture_output=True, text=True, + encoding="utf-8", + errors="replace", check=True, timeout=self._STOP_TIMEOUT_SECONDS, ) @@ -1927,6 +1982,8 @@ class LocalContainerBackend(SandboxBackend): [self._runtime, "inspect", "-f", "{{.State.Running}}", container_name], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=5, ) except subprocess.TimeoutExpired as exc: @@ -1952,6 +2009,8 @@ class LocalContainerBackend(SandboxBackend): [self._runtime, "port", container_name, "8080"], capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=5, ) if result.returncode == 0 and result.stdout.strip(): diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 2a6ce9852a..81bb4f85e5 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -2,6 +2,10 @@ Backend tests must preserve the runtime invariants they exercise without changing production execution topology. +The local sandbox's UTF-8 subprocess guard inspects each text-mode call with +`ast`, checking both `encoding` and `errors`; module-wide literal counts can +hide unpinned calls behind unrelated settings. + ## Router auth fixtures For owner-scoped route assertions, pass a stable `user_factory` and diff --git a/backend/tests/test_aio_sandbox_local_backend.py b/backend/tests/test_aio_sandbox_local_backend.py index e389c3a135..71b00d6737 100644 --- a/backend/tests/test_aio_sandbox_local_backend.py +++ b/backend/tests/test_aio_sandbox_local_backend.py @@ -1,13 +1,16 @@ +import ast import json import logging import os import socket import subprocess import time +from pathlib import Path from types import SimpleNamespace import pytest +from deerflow.community.aio_sandbox import local_backend as local_backend_module from deerflow.community.aio_sandbox.local_backend import ( LocalContainerBackend, _ContainerInspection, @@ -2349,3 +2352,57 @@ def test_start_container_preinitialized_image_can_drop_startup_caps(monkeypatch) assert not [arg for arg in captured_cmd if arg.startswith("--cap-add=")] security_opts = [captured_cmd[i + 1] for i, arg in enumerate(captured_cmd) if arg == "--security-opt"] assert "no-new-privileges" in security_opts + + +def test_docker_subprocess_calls_pin_utf8_decoding(monkeypatch): + """Locale-default text decoding silently loses docker CLI output on hosts + whose ANSI code page is not UTF-8 (e.g. cp936 Chinese Windows): the decode + error surfaces inside subprocess's reader thread, so ``stdout``/``stderr`` + come back ``None``, and JSON-parsing call sites either crash on ``None`` + or silently mis-parse. Every text-mode call in this module must therefore + pin ``encoding="utf-8"`` with ``errors="replace"``. + """ + seen: list[dict] = [] + + def fake_run(cmd, **kwargs): + seen.append(kwargs) + return SimpleNamespace(stdout='"Debian GNU/Linux 15"\n', stderr="", returncode=0) + + _docker_server_is_desktop.cache_clear() + monkeypatch.setattr("subprocess.run", fake_run) + try: + backend = LocalContainerBackend( + image="sandbox:latest", + base_port=8080, + container_prefix="sandbox", + config_mounts=[], + environment={}, + ) + assert backend._docker_server_is_desktop() is False + finally: + _docker_server_is_desktop.cache_clear() + + assert seen, "expected the docker detection path to shell out" + for kwargs in seen: + assert kwargs.get("encoding") == "utf-8", kwargs + assert kwargs.get("errors") == "replace", kwargs + + +def test_every_text_mode_subprocess_call_pins_utf8(): + """Check each text-mode subprocess call's encoding and error handling. + + A text-mode call without ``encoding=`` decodes with the platform's + preferred encoding, which loses output entirely on non-UTF-8 locales. + ``errors="replace"`` also keeps malformed bytes from discarding a stream. + """ + source = Path(local_backend_module.__file__).read_text(encoding="utf-8") + calls = [node for node in ast.walk(ast.parse(source)) if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) and isinstance(node.func.value, ast.Name) and node.func.value.id == "subprocess" and node.func.attr == "run"] + assert calls, "expected subprocess.run calls in the local backend" + for call in calls: + keywords = {keyword.arg: keyword.value for keyword in call.keywords} + text_mode = keywords.get("text") + if not isinstance(text_mode, ast.Constant) or text_mode.value is not True: + continue + for name, expected in (("encoding", "utf-8"), ("errors", "replace")): + value = keywords.get(name) + assert isinstance(value, ast.Constant) and value.value == expected, f"line {call.lineno}: text-mode subprocess.run must pass {name}={expected!r}"