mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
9e3589bb96
commit
f11be0a7bc
@ -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():
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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}"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user