diff --git a/README.md b/README.md index f988a2aa7..5d73af051 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ BenchFlow is a universal environment framework: it runs AI agents against task e - **Loop strategies** — wrap any agent in a `--loop-strategy` (`verify-retry`, `self-review`); every rollout captures a per-iteration reward + token trajectory, so you can plot capability against cost (can a cheap model + loops match an expensive one at equal token spend?) - **`task.md` tasks** — one file (YAML frontmatter + prompt body) replaces the split `task.toml` + `instruction.md` layout; author with `bench tasks init` / `check` / `migrate` / `export` - **Hosted environments** — run external PrimeIntellect / Verifiers environments through `--source-env`, without converting them to BenchFlow tasks -- **Sandboxes** — Docker locally, Daytona for parallel cloud runs (orphaned sandboxes auto-reaped at eval start), Modal for serverless/GPU-backed task environments +- **Sandboxes** — Docker locally, Apple Container on Apple Silicon Macs, Daytona for parallel cloud runs (orphaned sandboxes auto-reaped at eval start), Modal for serverless/GPU-backed task environments - **Hardened verifier** — defaults block BenchJack/Meerkat-style reward-hacking; tasks opt out per-feature - **Training-ready output** — every scored rollout emits ATIF (`trainer/atif.json`) and ADP (`trainer/adp.jsonl`) trajectory records next to the Verifiers/ORS (OpenReward) reward record diff --git a/docs/running-benchmarks.md b/docs/running-benchmarks.md index bdb0c8f0f..433b5469c 100644 --- a/docs/running-benchmarks.md +++ b/docs/running-benchmarks.md @@ -344,9 +344,18 @@ The **Harvey LAB harness** agent is special — it runs Harvey LAB's own agent l | Sandbox | Flag | Best for | |---------|------|----------| | Docker | `--sandbox docker` | Local development, small runs (≤10 tasks) | +| Apple Container | `--sandbox apple-container` | Local Apple Silicon macOS runs without Docker Desktop | | Daytona | `--sandbox daytona` | Cloud runs with concurrency (needs `DAYTONA_API_KEY`) | | Modal | `--sandbox modal` | Serverless, high concurrency (needs Modal auth) | +Apple Container requires Apple Container 1.1+ on Apple Silicon and runs the model +proxy inside each VM. It supports public-network, single-container arm64 tasks and +has no snapshot support. BenchFlow serializes Apple rollouts within each process +and blocks new VMs when the live `data.kalloc.1024` headroom is unsafe. Avoid +running concurrent BenchFlow processes, because the macOS allocation leak is +system-wide. Use Docker, Daytona, or Modal for `network_mode = "no-network"`, +multi-service, snapshot, or high-concurrency runs. + For large-scale runs (100+ tasks), use Daytona or Modal with high concurrency: ```bash diff --git a/src/benchflow/acp/runtime.py b/src/benchflow/acp/runtime.py index 7fb733d38..1443b874d 100644 --- a/src/benchflow/acp/runtime.py +++ b/src/benchflow/acp/runtime.py @@ -46,7 +46,12 @@ build_priv_drop_cmd, enforce_agent_egress_firewall, ) -from benchflow.sandbox.process import DaytonaProcess, DaytonaPtyProcess, DockerProcess +from benchflow.sandbox.process import ( + AppleContainerProcess, + DaytonaProcess, + DaytonaPtyProcess, + DockerProcess, +) from benchflow.trajectories._capture import _capture_session_trajectory # Re-exported for backwards compatibility — tests and downstream code @@ -574,6 +579,8 @@ async def connect_acp( try: if environment == "docker": live_proc = DockerProcess.from_sandbox_env(env) + elif environment == "apple-container": + live_proc = AppleContainerProcess.from_sandbox_env(env) elif environment == "daytona": transport_name = selected_acp_transport( agent=agent, diff --git a/src/benchflow/cli/main.py b/src/benchflow/cli/main.py index 092e1617d..7347b9190 100644 --- a/src/benchflow/cli/main.py +++ b/src/benchflow/cli/main.py @@ -588,7 +588,7 @@ def eval_run( ) -> None: """Run an evaluation — single task or batch. - Sandbox: docker, daytona, or modal. + Sandbox: docker, daytona, modal, or apple-container. """ _apply_dotenv_to_process_env() diff --git a/src/benchflow/continue_run/orchestrator.py b/src/benchflow/continue_run/orchestrator.py index 1e5b15df5..54b39bad2 100644 --- a/src/benchflow/continue_run/orchestrator.py +++ b/src/benchflow/continue_run/orchestrator.py @@ -37,7 +37,7 @@ sandbox_replay_base_url, ) from benchflow.contracts import AgentProtocolError, SandboxStartupFailure -from benchflow.sandbox.providers import OFF_BOX_MODEL_PROVIDERS +from benchflow.sandbox.providers import SANDBOX_MODEL_PROXY_PROVIDERS from benchflow.scenes import compile_scenes_to_steps from benchflow.trajectories.types import LLMExchange, redact_trajectory_text @@ -49,9 +49,9 @@ # OpenAI-compatible endpoint. _REPLAY_API_KEY = "sk-benchflow-replay" _REPLAY_MODEL = "openai/replay" -# Off-box-model providers (≡ non-docker) — derived from the canonical registry -# so this replay-routing subset can't drift from litellm_runtime's copy. -_SANDBOX_LOCAL_REPLAY_ENVIRONMENTS = OFF_BOX_MODEL_PROVIDERS +# Providers whose replay and model proxies must run inside the sandbox. This is +# the same placement contract used by the normal LiteLLM runtime. +_SANDBOX_LOCAL_REPLAY_ENVIRONMENTS = SANDBOX_MODEL_PROXY_PROVIDERS _PROXY_MODES = frozenset({"auto", "host", "sandbox"}) diff --git a/src/benchflow/providers/litellm_runtime.py b/src/benchflow/providers/litellm_runtime.py index 0d65fcb39..1686182f1 100644 --- a/src/benchflow/providers/litellm_runtime.py +++ b/src/benchflow/providers/litellm_runtime.py @@ -49,7 +49,7 @@ extract_usage_from_trajectory, trajectory_from_litellm_callback_log, ) -from benchflow.sandbox.providers import OFF_BOX_MODEL_PROVIDERS +from benchflow.sandbox.providers import SANDBOX_MODEL_PROXY_PROVIDERS from benchflow.trajectories._llm_capture import LiveLLMTrajectoryWriter from benchflow.trajectories.types import Trajectory from benchflow.usage_tracking import UsageTrackingConfig, usage_unavailable @@ -80,9 +80,10 @@ # GenerateContent format), so they talk to their provider directly and report # usage_source='unavailable'. ``oracle`` has no model at all. _NATIVE_PROTOCOL_AGENTS = frozenset({"oracle", "gemini"}) -# Providers whose model traffic exits the sandbox to the host proxy (≡ non-docker); -# derived from the canonical registry so it can't drift from the provider set. -_SANDBOX_LOCAL_ENVIRONMENTS = OFF_BOX_MODEL_PROVIDERS +# Providers whose mandatory LiteLLM proxy runs inside the sandbox. Keeping this +# placement policy in the canonical provider registry prevents a new backend from +# accidentally handing an in-sandbox agent a host-loopback endpoint. +_SANDBOX_LOCAL_ENVIRONMENTS = SANDBOX_MODEL_PROXY_PROVIDERS @dataclass(frozen=True) diff --git a/src/benchflow/sandbox/apple_container.py b/src/benchflow/sandbox/apple_container.py new file mode 100644 index 000000000..0693c5de2 --- /dev/null +++ b/src/benchflow/sandbox/apple_container.py @@ -0,0 +1,606 @@ +"""Apple Container sandbox backend using macOS Virtualization.framework. + +The backend targets Apple Container 1.1+ on Apple silicon. It delegates process +placement, detached lifecycle, and file transfer to the native CLI instead of +reimplementing those primitives with host subprocess state or shell pipelines. +""" + +from __future__ import annotations + +import asyncio +import json +import os +import platform +import re +import shlex +import shutil +import subprocess +import sys +from pathlib import Path, PurePosixPath + +from benchflow.sandbox._base import BaseSandbox, ExecResult, wrap_command_with_env_file + +_MIN_CONTAINER_VERSION = (1, 1, 0) +_KALLOC_SAFE_LIMIT = 3_000_000 +_KALLOC_MIN_HEADROOM = 200_000 +_DISK_MIN_GB = 5.0 +_STARTUP_TIMEOUT = 30 +_STOP_TIMEOUT = 30 + +# Apple Container currently leaks data.kalloc.1024 allocations across VM +# lifecycles. One active sandbox per BenchFlow process keeps the headroom check +# and launch atomic and matches the provider's documented safety envelope. +_RUN_SLOT = asyncio.Semaphore(1) + + +def _parse_version(value: str) -> tuple[int, int, int] | None: + match = re.search(r"(? tuple[int, int, int] | None: + """Read the Apple Container CLI version from its machine-readable output.""" + + try: + result = subprocess.run( + ["container", "system", "version", "--format", "json"], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode != 0: + return None + payload = json.loads(result.stdout) + except (json.JSONDecodeError, OSError, subprocess.TimeoutExpired): + return None + + rows = payload if isinstance(payload, list) else [payload] + for row in rows: + if not isinstance(row, dict): + continue + app_name = str(row.get("appName", "")).lower() + if app_name in {"container", "container cli"}: + return _parse_version(str(row.get("version", ""))) + return None + + +def _kalloc_headroom() -> tuple[int, int]: + """Return current in-use elements and safe headroom for data.kalloc.1024. + + macOS 26 ``zprint`` emits this row shape when headings are disabled: + + ``name elem_size cur_size max_size cur_elts max_elts cur_inuse ...`` + + ``cur_inuse`` is the live allocation count relevant to the Apple Container + VM leak. Keeping the real schema in this parser avoids the fictional + ``elems/maxelts`` fixture that previously selected the wrong column. + """ + + try: + result = subprocess.run( + ["zprint", "-H", "-L", "data.kalloc.1024"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode != 0: + return -1, -1 + for line in result.stdout.splitlines(): + parts = line.split() + if parts and parts[0] == "data.kalloc.1024" and len(parts) >= 7: + current_inuse = int(parts[6]) + return current_inuse, _KALLOC_SAFE_LIMIT - current_inuse + except (OSError, subprocess.TimeoutExpired, ValueError): + pass + return -1, -1 + + +def _require_kalloc_headroom() -> None: + current, headroom = _kalloc_headroom() + if current < 0: + raise RuntimeError( + "Unable to read data.kalloc.1024 usage from zprint. " + "Apple Container launch is blocked because VM headroom cannot be verified." + ) + if headroom < _KALLOC_MIN_HEADROOM: + raise RuntimeError( + "data.kalloc.1024 is near the safe Apple Container limit " + f"(in_use={current}, headroom={headroom}). Reboot your Mac before " + "starting another sandbox." + ) + + +def _disk_free_gb() -> float: + usage = shutil.disk_usage("/") + return usage.free / (1024**3) + + +async def _run_cli( + *args: str, + timeout: float | None = None, + stdin_data: bytes | None = None, +) -> ExecResult: + """Run an Apple ``container`` CLI command asynchronously.""" + + proc = await asyncio.create_subprocess_exec( + "container", + *args, + stdin=asyncio.subprocess.PIPE if stdin_data is not None else None, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + try: + stdout, stderr = await asyncio.wait_for( + proc.communicate(input=stdin_data), timeout=timeout + ) + except TimeoutError: + proc.kill() + await proc.wait() + raise + return ExecResult( + stdout=stdout.decode(errors="replace") if stdout else None, + stderr=stderr.decode(errors="replace") if stderr else None, + return_code=proc.returncode if proc.returncode is not None else 1, + ) + + +def _failure_output(result: ExecResult) -> str: + return (result.stderr or result.stdout or "no command output").strip() + + +class AppleContainerSandbox(BaseSandbox): + """Sandbox backend for Apple Container 1.1+ micro-VMs.""" + + _container_name: str | None = None + _holds_run_slot: bool = False + + @property + def is_mounted(self) -> bool: + return True + + @property + def sandbox_id(self) -> str | None: + return self._container_name + + @classmethod + def preflight(cls) -> None: + if sys.platform != "darwin": + raise RuntimeError( + "apple-container sandbox requires macOS (Virtualization.framework)" + ) + if platform.machine().lower() not in {"arm64", "aarch64"}: + raise RuntimeError("apple-container sandbox requires Apple silicon") + if not shutil.which("container"): + raise RuntimeError( + "container CLI not found. Install Apple Container 1.1 or newer: " + "https://github.com/apple/container/releases" + ) + + version = _container_cli_version() + if version is None: + raise RuntimeError( + "Unable to determine Apple Container CLI version with " + "`container system version --format json`." + ) + if version < _MIN_CONTAINER_VERSION: + actual = ".".join(str(part) for part in version) + required = ".".join(str(part) for part in _MIN_CONTAINER_VERSION) + raise RuntimeError( + f"Apple Container {required}+ is required; found {actual}. " + "Upgrade with `/usr/local/bin/update-container.sh`." + ) + + try: + result = subprocess.run( + ["container", "ls"], capture_output=True, text=True, timeout=10 + ) + except (OSError, subprocess.TimeoutExpired) as exc: + raise RuntimeError( + f"Unable to query the Apple Container service: {exc}" + ) from exc + if result.returncode != 0: + raise RuntimeError( + "container system not running. Start it with: container system start\n" + f"Error: {result.stderr.strip()}" + ) + + _require_kalloc_headroom() + free_gb = _disk_free_gb() + if free_gb < _DISK_MIN_GB: + raise RuntimeError( + f"Insufficient disk space ({free_gb:.1f}GB free, " + f"need >{_DISK_MIN_GB}GB for container images and VM storage." + ) + + def _validate_definition(self) -> None: + dockerfile = self.environment_dir / "Dockerfile" + if not dockerfile.exists() and not self.task_env_config.docker_image: + raise ValueError( + f"No Dockerfile found in {self.environment_dir} and no " + "docker_image specified in task config." + ) + if not self.task_env_config.allow_internet: + raise ValueError( + "apple-container does not currently enforce no-network sandboxing. " + "Use docker, daytona, or modal for tasks that require " + "environment.network_mode='no-network'." + ) + + def _image_tag(self) -> str: + safe_name = re.sub(r"[^a-zA-Z0-9_.-]+", "_", self.environment_name) + return f"bf__{safe_name}" + + async def _resolve_image(self, *, force_build: bool) -> str: + dockerfile = self.environment_dir / "Dockerfile" + if self.task_env_config.docker_image and not force_build: + return self.task_env_config.docker_image + if not dockerfile.exists(): + raise ValueError( + f"No Dockerfile found in {self.environment_dir}; " + "cannot force-build apple-container image." + ) + + args = ["build"] + if force_build: + args.append("--no-cache") + args.extend( + [ + "--platform", + "linux/arm64", + "-f", + str(dockerfile), + "-t", + self._image_tag(), + str(self.environment_dir), + ] + ) + result = await _run_cli( + *args, + timeout=self.task_env_config.build_timeout_sec, + ) + if result.return_code != 0: + raise RuntimeError( + f"container build failed (exit {result.return_code}):\n" + f"{_failure_output(result)}" + ) + return self._image_tag() + + def _release_run_slot(self) -> None: + if self._holds_run_slot: + self._holds_run_slot = False + _RUN_SLOT.release() + + async def start(self, force_build: bool) -> None: + if self._holds_run_slot or self._container_name is not None: + raise RuntimeError("Apple Container sandbox is already started.") + + await _RUN_SLOT.acquire() + self._holds_run_slot = True + try: + _require_kalloc_headroom() + image = await self._resolve_image(force_build=force_build) + safe_session = re.sub(r"[^a-zA-Z0-9_.-]+", "_", self.session_id) + self._container_name = f"bf_{safe_session}"[:63] + + cmd_args = [ + "run", + "--detach", + "--name", + self._container_name, + "--platform", + "linux/arm64", + "-c", + str(self.task_env_config.cpus or 2), + "-m", + f"{self.task_env_config.memory_mb or 2048}M", + ] + + if self.rollout_paths: + logs_dir = self.rollout_paths.rollout_dir + for sub in ("verifier", "agent", "artifacts"): + os.makedirs(logs_dir / sub, exist_ok=True) + os.chmod(logs_dir / sub, 0o777) + cmd_args.extend( + ["--mount", f"type=bind,source={logs_dir},target=/logs"] + ) + + cmd_args.extend( + [ + "--entrypoint", + "/bin/sh", + image, + "-c", + "sleep infinity || while :; do sleep 3600; done", + ] + ) + result = await _run_cli( + *cmd_args, + timeout=self.task_env_config.build_timeout_sec, + ) + if result.return_code != 0: + raise RuntimeError( + f"container run failed (exit {result.return_code}):\n" + f"{_failure_output(result)}" + ) + + deadline = asyncio.get_running_loop().time() + _STARTUP_TIMEOUT + while asyncio.get_running_loop().time() < deadline: + try: + ready = await _run_cli( + "exec", self._container_name, "true", timeout=5 + ) + if ready.return_code == 0: + self.logger.info( + "Started container %s (image=%s)", + self._container_name, + image, + ) + return + except (TimeoutError, OSError): + pass + await asyncio.sleep(0.5) + + logs = await _run_cli("logs", self._container_name, timeout=10) + raise RuntimeError( + f"Container {self._container_name} did not become ready within " + f"{_STARTUP_TIMEOUT}s: {_failure_output(logs)}" + ) + except BaseException: + await self._force_cleanup() + raise + + async def exec( + self, + command: str, + cwd: str | None = None, + env: dict[str, str] | None = None, + timeout_sec: int | None = None, + user: str | int | None = None, + service: str = "main", + ) -> ExecResult: + if service != "main": + raise ValueError( + "apple-container is a single-container backend; " + f"service={service!r} is not supported." + ) + if not self._container_name: + raise RuntimeError("Container not started. Call start() first.") + + wrapped = command + merged_env = self._merge_env(env) + if merged_env: + wrapped = wrap_command_with_env_file( + merged_env, wrapped, env_path_prefix="/tmp/.bf_env_" + ) + + args = ["exec"] + if cwd: + args.extend(["--workdir", cwd]) + resolved_user = self._resolve_user(user) + if resolved_user is not None: + args.extend(["--user", str(resolved_user)]) + args.extend([self._container_name, "sh", "-c", wrapped]) + + try: + return await _run_cli(*args, timeout=timeout_sec) + except TimeoutError: + self.logger.error( + "exec timed out after %ss, removing container", timeout_sec + ) + await self._force_cleanup() + raise RuntimeError( + f"Command timed out after {timeout_sec}s: {command[:100]}" + ) from None + + async def _copy( + self, source: str, destination: str, *, operation: str, timeout: float = 120 + ) -> None: + result = await _run_cli("cp", source, destination, timeout=timeout) + if result.return_code != 0: + raise RuntimeError(f"{operation} failed: {_failure_output(result)}") + + async def upload_file(self, source_path: Path | str, target_path: str) -> None: + source = Path(source_path) + if not self._container_name: + raise RuntimeError("Container not started.") + + host_path = self._mounted_host_path(target_path) + if host_path is not None: + host_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, host_path) + return + + target_parent = str(PurePosixPath(target_path).parent) + prep = await self.exec( + f"mkdir -p {shlex.quote(target_parent)}", timeout_sec=30, user="root" + ) + if prep.return_code != 0: + raise RuntimeError( + f"upload_file destination prep failed: {_failure_output(prep)}" + ) + await self._copy( + str(source), + f"{self._container_name}:{target_path}", + operation="upload_file", + ) + + async def upload_dir( + self, source_dir: Path | str, target_dir: str, service: str = "main" + ) -> None: + if service != "main": + raise ValueError( + "apple-container is single-container; service must be 'main'." + ) + source = Path(source_dir) + if not source.is_dir(): + raise FileNotFoundError(f"Source directory {source} does not exist") + if not self._container_name: + raise RuntimeError("Container not started.") + + host_path = self._mounted_host_path(target_dir) + if host_path is not None: + if host_path.exists(): + shutil.rmtree(host_path) + shutil.copytree(source, host_path) + return + + prep = await self.exec( + f"mkdir -p {shlex.quote(target_dir)}", timeout_sec=30, user="root" + ) + if prep.return_code != 0: + raise RuntimeError( + f"upload_dir destination prep failed: {_failure_output(prep)}" + ) + # Apple ``container cp source/ existing-target`` nests ``source`` under + # the target. BenchFlow's upload_dir contract copies the directory's + # contents, so copy each immediate child through the native primitive. + # This includes dotfiles and preserves nested subtrees without a shell + # archive/base64 transport. + destination = f"{self._container_name}:{target_dir.rstrip('/')}/" + for child in sorted(source.iterdir(), key=lambda path: path.name): + await self._copy( + str(child), + destination, + operation=f"upload_dir ({child.name})", + ) + + async def download_file(self, source_path: str, target_path: Path | str) -> None: + target = Path(target_path) + if not self._container_name: + raise RuntimeError("Container not started.") + + host_path = self._mounted_host_path(source_path) + if host_path is not None: + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(host_path, target) + return + + target.parent.mkdir(parents=True, exist_ok=True) + await self._copy( + f"{self._container_name}:{source_path}", + str(target), + operation="download_file", + ) + + async def download_dir( + self, source_dir: str, target_dir: Path | str, service: str = "main" + ) -> None: + if service != "main": + raise ValueError( + "apple-container is single-container; service must be 'main'." + ) + target = Path(target_dir) + if not self._container_name: + raise RuntimeError("Container not started.") + + host_path = self._mounted_host_path(source_dir) + if host_path is not None: + if target.exists(): + shutil.rmtree(target) + shutil.copytree(host_path, target) + return + + if target.exists(): + shutil.rmtree(target) + target.parent.mkdir(parents=True, exist_ok=True) + await self._copy( + f"{self._container_name}:{source_dir}", + str(target), + operation="download_dir", + ) + + async def stop(self, delete: bool) -> None: + name = self._container_name + try: + if not name: + return + try: + stopped = await _run_cli( + "stop", "--time", "5", name, timeout=_STOP_TIMEOUT + ) + if stopped.return_code != 0: + self.logger.warning( + "Failed to stop Apple container %s cleanly: %s", + name, + _failure_output(stopped), + ) + except (TimeoutError, OSError) as exc: + self.logger.warning( + "Apple container %s stop failed; continuing cleanup: %s", + name, + exc, + ) + + if delete: + try: + removed = await _run_cli("rm", "--force", name, timeout=10) + if removed.return_code != 0: + self.logger.warning( + "Failed to remove Apple container %s: %s", + name, + _failure_output(removed), + ) + except (TimeoutError, OSError) as exc: + self.logger.warning( + "Apple container %s removal failed: %s", name, exc + ) + self._container_name = None + self.logger.info("Stopped container %s", name) + finally: + self._release_run_slot() + + async def _force_cleanup(self) -> None: + """Best-effort atomic cleanup after startup or execution failure.""" + + name = self._container_name + try: + if name: + try: + result = await _run_cli("rm", "--force", name, timeout=10) + if result.return_code != 0: + self.logger.warning( + "Forced Apple container cleanup failed for %s: %s", + name, + _failure_output(result), + ) + except (TimeoutError, OSError) as exc: + self.logger.warning( + "Forced Apple container cleanup failed for %s: %s", name, exc + ) + finally: + self._container_name = None + self._release_run_slot() + + def _mounted_host_path(self, container_path: str) -> Path | None: + """Map a container path to a host path when it is under ``/logs``.""" + + if not self.rollout_paths: + return None + path = PurePosixPath(container_path) + prefix = PurePosixPath("/logs") + if not path.is_absolute(): + return None + if path == prefix: + rel_parts: tuple[str, ...] = () + elif path.parts[: len(prefix.parts)] == prefix.parts: + rel_parts = path.parts[len(prefix.parts) :] + else: + return None + if any(part == ".." for part in rel_parts): + raise ValueError(f"Unsafe mounted path escapes /logs: {container_path!r}") + + host_root = self.rollout_paths.rollout_dir.resolve() + candidate = host_root.joinpath(*rel_parts).resolve(strict=False) + try: + candidate.relative_to(host_root) + except ValueError as exc: + raise ValueError( + f"Unsafe mounted path escapes /logs: {container_path!r}" + ) from exc + return candidate diff --git a/src/benchflow/sandbox/process.py b/src/benchflow/sandbox/process.py index 6e7a48b50..3d7b6d2af 100644 --- a/src/benchflow/sandbox/process.py +++ b/src/benchflow/sandbox/process.py @@ -1,9 +1,10 @@ """Live stdio connection to a process inside a sandbox. Provides a bidirectional pipe (send lines in, read lines out) needed for -ACP agents running inside containers. Two implementations: +ACP agents running inside containers. Implementations: - DockerProcess: uses `docker compose exec -i` (local Docker) +- AppleContainerProcess: uses `container exec -i` (Apple Container) - DaytonaProcess: uses SSH to a Daytona sandbox """ @@ -389,6 +390,90 @@ async def start( ) +class AppleContainerProcess(LiveProcess): + """Live stdin/stdout through Apple Container's native exec transport.""" + + def __init__(self, container_name: str): + self._container_name = container_name + self._env_path = f"/tmp/.benchflow_agent_env_{uuid.uuid4().hex[:16]}" + + @classmethod + def from_sandbox_env(cls, env: Any) -> "AppleContainerProcess": + """Create from a started AppleContainerSandbox.""" + + container_name = getattr(env, "_container_name", None) + if not isinstance(container_name, str) or not container_name: + raise RuntimeError("Apple Container sandbox not started") + return cls(container_name) + + async def _write_env_to_container(self, env: dict[str, str]) -> None: + invalid = [key for key in env if not _ENV_KEY_RE.match(key)] + if invalid: + raise ValueError( + "Invalid environment variable name(s): " + ", ".join(sorted(invalid)) + ) + + lines = "".join( + f"export {key}={shlex.quote(value)}\n" for key, value in env.items() + ) + env_path = shlex.quote(self._env_path) + proc = await asyncio.create_subprocess_exec( + "container", + "exec", + "--interactive", + "--user", + "root", + self._container_name, + "sh", + "-c", + f"cat > {env_path} && chmod 600 {env_path}", + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + try: + _, stderr = await asyncio.wait_for( + proc.communicate(lines.encode()), timeout=30 + ) + except TimeoutError: + proc.kill() + await proc.wait() + raise + if proc.returncode != 0: + raise RuntimeError( + "Failed to write agent env in Apple container " + f"(rc={proc.returncode}): {stderr.decode(errors='replace')[:500]}" + ) + + async def start( + self, + command: str, + env: dict[str, str] | None = None, + cwd: str | None = None, + ) -> None: + if env: + await self._write_env_to_container(env) + env_path = shlex.quote(self._env_path) + command = f". {env_path} && rm -f {env_path} && {command}" + + args = ["container", "exec", "--interactive"] + if cwd: + args.extend(["--workdir", cwd]) + args.extend([self._container_name, "bash", "-c", command]) + self._process = await asyncio.create_subprocess_exec( + *args, + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + limit=_BUFFER_LIMIT, + ) + logger.info( + "Apple Container process started (pid=%s, container=%s)", + self._process.pid, + self._container_name, + ) + + class DaytonaProcess(LiveProcess): """Live stdin/stdout via SSH to a Daytona sandbox. diff --git a/src/benchflow/sandbox/providers.py b/src/benchflow/sandbox/providers.py index d0fda952c..ed90c21fc 100644 --- a/src/benchflow/sandbox/providers.py +++ b/src/benchflow/sandbox/providers.py @@ -19,6 +19,14 @@ from __future__ import annotations from dataclasses import dataclass +from enum import StrEnum + + +class ModelProxyLocation(StrEnum): + """Where BenchFlow runs the mandatory LiteLLM proxy for a sandbox.""" + + HOST = "host" + SANDBOX = "sandbox" @dataclass(frozen=True) @@ -26,15 +34,34 @@ class SandboxProvider: """One sandbox backend and the facts that were previously duplicated.""" name: str - extra: str | None # pip/uv optional-dependency extra; None for built-in docker - off_box_model: bool # model traffic exits the sandbox → host proxy (not docker) + extra: str | None # pip/uv optional-dependency extra; None when none is needed + model_proxy: ModelProxyLocation + + @property + def off_box_model(self) -> bool: + """Backward-compatible alias for the old, ambiguously named flag.""" + + return self.model_proxy is ModelProxyLocation.SANDBOX # Ordered, docker-first. This tuple is the ONLY place the set is spelled out. _PROVIDERS: tuple[SandboxProvider, ...] = ( - SandboxProvider("docker", extra=None, off_box_model=False), - SandboxProvider("daytona", extra="sandbox-daytona", off_box_model=True), - SandboxProvider("modal", extra="sandbox-modal", off_box_model=True), + SandboxProvider("docker", extra=None, model_proxy=ModelProxyLocation.HOST), + SandboxProvider( + "daytona", + extra="sandbox-daytona", + model_proxy=ModelProxyLocation.SANDBOX, + ), + SandboxProvider( + "modal", + extra="sandbox-modal", + model_proxy=ModelProxyLocation.SANDBOX, + ), + SandboxProvider( + "apple-container", + extra=None, + model_proxy=ModelProxyLocation.SANDBOX, + ), ) PROVIDERS_BY_NAME: dict[str, SandboxProvider] = {p.name: p for p in _PROVIDERS} @@ -47,10 +74,12 @@ class SandboxProvider: OPTIONAL_SANDBOX_EXTRAS: dict[str, str] = { p.name: p.extra for p in _PROVIDERS if p.extra is not None } -#: Providers whose model traffic must reach the host proxy off-box (≡ non-docker). -OFF_BOX_MODEL_PROVIDERS: frozenset[str] = frozenset( - p.name for p in _PROVIDERS if p.off_box_model +#: Providers whose LiteLLM proxy must run inside the sandbox. +SANDBOX_MODEL_PROXY_PROVIDERS: frozenset[str] = frozenset( + p.name for p in _PROVIDERS if p.model_proxy is ModelProxyLocation.SANDBOX ) +# Backward-compatible import alias. New code uses the literal placement contract. +OFF_BOX_MODEL_PROVIDERS = SANDBOX_MODEL_PROXY_PROVIDERS def is_known_provider(name: str) -> bool: @@ -59,7 +88,7 @@ def is_known_provider(name: str) -> bool: def provider_extra(name: str) -> str | None: - """The optional-dependency extra for ``name`` (None for docker/unknown).""" + """The optional-dependency extra for ``name`` (None if absent/unknown).""" p = PROVIDERS_BY_NAME.get(name) return p.extra if p else None diff --git a/src/benchflow/sandbox/setup.py b/src/benchflow/sandbox/setup.py index d112e0191..1f5bdc28a 100644 --- a/src/benchflow/sandbox/setup.py +++ b/src/benchflow/sandbox/setup.py @@ -781,6 +781,18 @@ def _create_sandbox_environment( task_env_config=env_config, persistent_env=manifest_env or None, ) + elif sandbox_type == "apple-container": + from benchflow.sandbox.apple_container import AppleContainerSandbox + + AppleContainerSandbox.preflight() + return AppleContainerSandbox( + environment_dir=environment_dir, + environment_name=task_path.name, + session_id=rollout_name, + rollout_paths=rollout_paths, + task_env_config=env_config, + persistent_env=manifest_env or None, + ) else: raise ValueError( f"Unknown sandbox_type: {sandbox_type!r} (use {providers_phrase(quote=True)})" diff --git a/src/benchflow/task/runtime_capabilities.py b/src/benchflow/task/runtime_capabilities.py index 24b3f442d..2afd0c114 100644 --- a/src/benchflow/task/runtime_capabilities.py +++ b/src/benchflow/task/runtime_capabilities.py @@ -261,6 +261,13 @@ def _append_network_issue( mode: NetworkMode | None, sandbox: str, ) -> None: + if sandbox == "apple-container" and mode == NetworkMode.NO_NETWORK: + _issue( + unsupported, + path=path, + reason="network_mode='no-network' is not enforced by apple-container", + sandbox=sandbox, + ) if mode == NetworkMode.ALLOWLIST: _issue( unsupported, diff --git a/tests/continue_run/test_orchestrator.py b/tests/continue_run/test_orchestrator.py index 13a92448d..de67ade4d 100644 --- a/tests/continue_run/test_orchestrator.py +++ b/tests/continue_run/test_orchestrator.py @@ -39,9 +39,10 @@ def test_build_agent_env_points_at_proxy(): def test_select_proxy_mode_uses_sandbox_for_remote_environments(): - """Guards PR #648 follow-up: Daytona cannot reach host-loopback replay.""" + """Guards PR #648 and #936 against unreachable host-loopback replay.""" assert select_proxy_mode("auto", "daytona") == "sandbox" assert select_proxy_mode("auto", "modal") == "sandbox" + assert select_proxy_mode("auto", "apple-container") == "sandbox" assert select_proxy_mode("auto", "docker") == "host" assert select_proxy_mode("host", "daytona") == "host" diff --git a/tests/test_acp.py b/tests/test_acp.py index 55949dbb7..97f51790c 100644 --- a/tests/test_acp.py +++ b/tests/test_acp.py @@ -1216,6 +1216,45 @@ async def test_claude_litellm_env_owns_model_selection(self, tmp_path): # LiteLLM VIA_ENV owns model selection -> no ACP set_model or config option. mock_acp.set_config_option.assert_not_awaited() + @pytest.mark.asyncio + async def test_apple_container_uses_native_live_process(self, tmp_path): + """Guards PR #936 against treating Apple Container as Daytona.""" + + from benchflow.acp.runtime import connect_acp + + mock_acp = self._make_mocks() + mock_env = MagicMock() + mock_env.exec = AsyncMock(return_value=MagicMock(return_code=1, stdout="")) + live_process = MagicMock() + + with ( + patch( + "benchflow.acp.runtime.AppleContainerProcess.from_sandbox_env", + return_value=live_process, + ) as apple_process, + patch( + "benchflow.acp.runtime.DaytonaProcess.from_sandbox_env", + new_callable=AsyncMock, + ) as daytona_process, + patch("benchflow.acp.runtime.ContainerTransport") as transport, + patch("benchflow.acp.runtime.ACPClient", return_value=mock_acp), + ): + await connect_acp( + env=mock_env, + agent="codex-acp", + agent_launch="codex-acp", + agent_env={}, + sandbox_user=None, + model=None, + rollout_dir=tmp_path, + environment="apple-container", + agent_cwd="/root", + ) + + apple_process.assert_called_once_with(mock_env) + daytona_process.assert_not_awaited() + assert transport.call_args.kwargs["container_process"] is live_process + @pytest.mark.asyncio async def test_daytona_dind_uses_pty_transport(self, tmp_path): """Daytona compose tasks use PTY transport to avoid SSH pipe-closed failures.""" diff --git a/tests/test_apple_container_sandbox.py b/tests/test_apple_container_sandbox.py new file mode 100644 index 000000000..0b57a5b0b --- /dev/null +++ b/tests/test_apple_container_sandbox.py @@ -0,0 +1,578 @@ +"""Tests for the Apple Container sandbox backend. + +Unit tests exercise argv and lifecycle contracts without requiring macOS. The +integration test at the bottom is gated on a real Apple Container installation. +""" + +from __future__ import annotations + +import asyncio +import json +import shutil +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from benchflow.sandbox import apple_container as apple_mod +from benchflow.sandbox._base import ExecResult +from benchflow.sandbox.apple_container import ( + AppleContainerSandbox, + _container_cli_version, + _kalloc_headroom, + _parse_version, +) +from benchflow.task.config import NetworkMode, SandboxConfig +from benchflow.task.paths import RolloutPaths + + +@pytest.fixture(autouse=True) +def isolated_run_slot(monkeypatch): + """Give each test an unclaimed process-local lifecycle slot.""" + + monkeypatch.setattr(apple_mod, "_RUN_SLOT", asyncio.Semaphore(1)) + + +@pytest.fixture +def make_sandbox(tmp_path): + sandboxes: list[AppleContainerSandbox] = [] + + def make( + *, + image: str | None = None, + skills_dir: str | None = None, + allow_internet: bool = True, + network_mode: NetworkMode = NetworkMode.PUBLIC, + session_id: str = "sess-001", + ) -> AppleContainerSandbox: + index = len(sandboxes) + env_dir = tmp_path / f"environment-{index}" + env_dir.mkdir() + (env_dir / "Dockerfile").write_text("FROM ubuntu:24.04\nRUN echo hi\n") + rollout_dir = tmp_path / f"rollout-{index}" + paths = RolloutPaths(rollout_dir) + config = SandboxConfig( + cpus=2, + memory_mb=1024, + docker_image=image, + skills_dir=skills_dir, + build_timeout_sec=60, + allow_internet=allow_internet, + network_mode=network_mode, + ) + with patch.object(AppleContainerSandbox, "preflight"): + sandbox = AppleContainerSandbox( + environment_dir=env_dir, + environment_name="test-task", + session_id=session_id, + rollout_paths=paths, + task_env_config=config, + ) + sandboxes.append(sandbox) + return sandbox + + return make + + +def _success(stdout: str = "") -> ExecResult: + return ExecResult(stdout=stdout, stderr=None, return_code=0) + + +def _started(make_sandbox) -> AppleContainerSandbox: + sandbox = make_sandbox() + sandbox._container_name = "bf_sess-001" + return sandbox + + +class TestVersionAndPreflight: + def test_parse_version_accepts_release_text(self): + """Guards PR #936 against rejecting Apple Container release output.""" + + assert _parse_version("container CLI version 1.1.0") == (1, 1, 0) + assert _parse_version("invalid") is None + + @pytest.mark.parametrize( + "payload", + [ + {"appName": "container", "version": "1.1.0"}, + [ + {"appName": "container-apiserver", "version": "1.1.0"}, + {"appName": "container CLI", "version": "1.1.0"}, + ], + ], + ) + def test_cli_version_handles_old_and_current_json_shapes(self, payload): + """Guards PR #936 across Apple Container version output shape changes.""" + + completed = MagicMock(returncode=0, stdout=json.dumps(payload)) + with patch("subprocess.run", return_value=completed): + assert _container_cli_version() == (1, 1, 0) + + def test_kalloc_parser_reads_live_cur_inuse_column(self): + """Guards PR #936 against selecting cur-size instead of cur-inuse.""" + + output = ( + "data.kalloc.1024 1024 0K 0K 0 0 1934 0K 0\n" + "data.kalloc.2048 2048 0K 0K 0 0 99 0K 0\n" + ) + completed = MagicMock(returncode=0, stdout=output) + with patch("subprocess.run", return_value=completed) as run: + current, headroom = _kalloc_headroom() + assert current == 1934 + assert headroom == 3_000_000 - 1934 + run.assert_called_once_with( + ["zprint", "-H", "-L", "data.kalloc.1024"], + capture_output=True, + text=True, + timeout=5, + ) + + def test_kalloc_parser_fails_closed_on_unknown_shape(self): + """Guards PR #936 against trusting an unknown zprint schema.""" + + completed = MagicMock(returncode=0, stdout="unrecognized output\n") + with patch("subprocess.run", return_value=completed): + assert _kalloc_headroom() == (-1, -1) + + def test_preflight_rejects_non_darwin(self, monkeypatch): + """Guards PR #936 against selecting Apple Container off macOS.""" + + monkeypatch.setattr(apple_mod.sys, "platform", "linux") + with pytest.raises(RuntimeError, match="requires macOS"): + AppleContainerSandbox.preflight() + + def test_preflight_rejects_non_apple_silicon(self, monkeypatch): + """Guards PR #936 against selecting Apple Container on Intel Macs.""" + + monkeypatch.setattr(apple_mod.sys, "platform", "darwin") + monkeypatch.setattr(apple_mod.platform, "machine", lambda: "x86_64") + with pytest.raises(RuntimeError, match="requires Apple silicon"): + AppleContainerSandbox.preflight() + + def test_preflight_rejects_old_cli(self, monkeypatch): + """Guards PR #936 against using unsupported native CLI behavior.""" + + monkeypatch.setattr(apple_mod.sys, "platform", "darwin") + monkeypatch.setattr(apple_mod.platform, "machine", lambda: "arm64") + monkeypatch.setattr(apple_mod.shutil, "which", lambda _name: "/bin/container") + monkeypatch.setattr(apple_mod, "_container_cli_version", lambda: (1, 0, 0)) + with pytest.raises(RuntimeError, match=r"1\.1\.0\+ is required"): + AppleContainerSandbox.preflight() + + def test_preflight_fails_closed_when_kalloc_is_unreadable(self, monkeypatch): + """Guards PR #936 against launching when VM leak headroom is unknown.""" + + monkeypatch.setattr(apple_mod.sys, "platform", "darwin") + monkeypatch.setattr(apple_mod.platform, "machine", lambda: "arm64") + monkeypatch.setattr(apple_mod.shutil, "which", lambda _name: "/bin/container") + monkeypatch.setattr(apple_mod, "_container_cli_version", lambda: (1, 1, 0)) + monkeypatch.setattr( + apple_mod.subprocess, + "run", + lambda *_args, **_kwargs: MagicMock(returncode=0, stderr=""), + ) + monkeypatch.setattr(apple_mod, "_kalloc_headroom", lambda: (-1, -1)) + with pytest.raises(RuntimeError, match="cannot be verified"): + AppleContainerSandbox.preflight() + + def test_preflight_rejects_low_kalloc_headroom(self, monkeypatch): + """Guards PR #936 against crossing Apple's documented crash region.""" + + monkeypatch.setattr(apple_mod.sys, "platform", "darwin") + monkeypatch.setattr(apple_mod.platform, "machine", lambda: "arm64") + monkeypatch.setattr(apple_mod.shutil, "which", lambda _name: "/bin/container") + monkeypatch.setattr(apple_mod, "_container_cli_version", lambda: (1, 1, 0)) + monkeypatch.setattr( + apple_mod.subprocess, + "run", + lambda *_args, **_kwargs: MagicMock(returncode=0, stderr=""), + ) + monkeypatch.setattr(apple_mod, "_kalloc_headroom", lambda: (2_850_000, 150_000)) + with pytest.raises(RuntimeError, match="Reboot your Mac"): + AppleContainerSandbox.preflight() + + +class TestDefinitionAndBuild: + def test_rejects_missing_dockerfile_and_image(self, make_sandbox, tmp_path): + """Guards PR #936 against launching without an image definition.""" + + sandbox = make_sandbox(image="ubuntu:24.04") + sandbox.environment_dir.joinpath("Dockerfile").unlink() + sandbox.task_env_config.docker_image = None + with pytest.raises(ValueError, match="No Dockerfile"): + sandbox._validate_definition() + + def test_rejects_no_network(self, make_sandbox): + """Guards PR #936 against claiming unenforced network isolation.""" + + with pytest.raises(ValueError, match="does not currently enforce no-network"): + make_sandbox( + allow_internet=False, + network_mode=NetworkMode.NO_NETWORK, + ) + + @pytest.mark.asyncio + async def test_prebuilt_image_wins_without_force_build(self, make_sandbox): + """Guards PR #936 against rebuilding an explicitly configured image.""" + + sandbox = make_sandbox(image="ubuntu:24.04") + with patch.object(apple_mod, "_run_cli", new_callable=AsyncMock) as run: + assert await sandbox._resolve_image(force_build=False) == "ubuntu:24.04" + run.assert_not_awaited() + + @pytest.mark.asyncio + @pytest.mark.parametrize("force_build", [False, True]) + async def test_build_is_native_arm64_and_cache_policy_is_explicit( + self, make_sandbox, force_build + ): + """Guards PR #936 against forced cache misses and amd64 VM failures.""" + + sandbox = make_sandbox() + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + image = await sandbox._resolve_image(force_build=force_build) + assert image == "bf__test-task" + call = run.await_args + assert call is not None + args = call.args + assert args[0] == "build" + assert args[args.index("--platform") + 1] == "linux/arm64" + assert ("--no-cache" in args) is force_build + + +class TestStartAndLifecycle: + @pytest.mark.asyncio + async def test_start_uses_detach_and_only_mounts_logs(self, make_sandbox): + """Guards PR #936 against task-source and skills host mutation.""" + + sandbox = make_sandbox(skills_dir="/skills") + with ( + patch.object(apple_mod, "_require_kalloc_headroom"), + patch.object( + sandbox, + "_resolve_image", + new_callable=AsyncMock, + return_value="ubuntu:24.04", + ), + patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run, + ): + await sandbox.start(force_build=False) + + launch = run.await_args_list[0].args + assert launch[:2] == ("run", "--detach") + joined = "\n".join(launch) + assert "target=/logs" in joined + assert "target=/app" not in joined + assert "/skills" not in joined + assert "--platform\nlinux/arm64" in joined + + @pytest.mark.asyncio + async def test_start_does_not_put_provider_secrets_on_run_argv(self, make_sandbox): + """Guards PR #936 against exposing model credentials in host process argv.""" + + sandbox = make_sandbox() + sandbox._persistent_env = {"API_KEY": "sk-secret-123"} + with ( + patch.object(apple_mod, "_require_kalloc_headroom"), + patch.object( + sandbox, + "_resolve_image", + new_callable=AsyncMock, + return_value="ubuntu:24.04", + ), + patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run, + ): + await sandbox.start(force_build=False) + launch = run.await_args_list[0].args + assert "sk-secret-123" not in "\n".join(launch) + assert "-e" not in launch + + @pytest.mark.asyncio + async def test_start_failure_releases_process_slot(self, make_sandbox): + """Guards PR #936 against permanently wedging later Apple rollouts.""" + + sandbox = make_sandbox() + with ( + patch.object(apple_mod, "_require_kalloc_headroom"), + patch.object( + sandbox, + "_resolve_image", + new_callable=AsyncMock, + return_value="ubuntu:24.04", + ), + patch.object( + apple_mod, + "_run_cli", + new_callable=AsyncMock, + return_value=ExecResult( + stdout=None, stderr="launch failed", return_code=1 + ), + ), + pytest.raises(RuntimeError, match="launch failed"), + ): + await sandbox.start(force_build=False) + assert not apple_mod._RUN_SLOT.locked() + assert sandbox.sandbox_id is None + + @pytest.mark.asyncio + async def test_only_one_sandbox_is_active_per_process(self, make_sandbox): + """Guards PR #936 by making the kalloc safety limit enforceable.""" + + first = make_sandbox(session_id="first") + second = make_sandbox(session_id="second") + with ( + patch.object(apple_mod, "_require_kalloc_headroom"), + patch.object( + AppleContainerSandbox, + "_resolve_image", + new_callable=AsyncMock, + return_value="ubuntu:24.04", + ), + patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ), + ): + await first.start(force_build=False) + second_start = asyncio.create_task(second.start(force_build=False)) + await asyncio.sleep(0) + assert not second_start.done() + await first.stop(delete=True) + await asyncio.wait_for(second_start, timeout=1) + await second.stop(delete=True) + assert not apple_mod._RUN_SLOT.locked() + + @pytest.mark.asyncio + async def test_stop_uses_native_stop_and_force_remove(self, make_sandbox): + """Guards PR #936 against leaking VMs or stopping global BuildKit.""" + + sandbox = _started(make_sandbox) + sandbox._holds_run_slot = True + await apple_mod._RUN_SLOT.acquire() + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.stop(delete=True) + calls = [call.args for call in run.await_args_list] + assert ("stop", "--time", "5", "bf_sess-001") in calls + assert ("rm", "--force", "bf_sess-001") in calls + assert all("buildkit" not in call for call in calls) + assert sandbox.sandbox_id is None + assert not apple_mod._RUN_SLOT.locked() + + +class TestExec: + @pytest.mark.asyncio + async def test_exec_uses_native_workdir_and_user_flags(self, make_sandbox): + """Guards PR #936 against shell-injected cwd/user and missing native flags.""" + + sandbox = _started(make_sandbox) + hostile_user = "worker; touch /tmp/pwned" + hostile_cwd = "/app/a path; touch /tmp/pwned" + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.exec("printf ok", cwd=hostile_cwd, user=hostile_user) + run.assert_awaited_once_with( + "exec", + "--workdir", + hostile_cwd, + "--user", + hostile_user, + "bf_sess-001", + "sh", + "-c", + "printf ok", + timeout=None, + ) + + @pytest.mark.asyncio + async def test_exec_preserves_numeric_user(self, make_sandbox): + """Guards PR #936 against dropping valid numeric user identities.""" + + sandbox = _started(make_sandbox) + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.exec("id", user=1000) + call = run.await_args + assert call is not None + assert call.args[1:3] == ("--user", "1000") + + @pytest.mark.asyncio + async def test_exec_redacts_environment_secret(self, make_sandbox): + """Guards PR #936 against exposing exec secrets in host argv.""" + + sandbox = _started(make_sandbox) + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.exec("run.sh", env={"API_KEY": "sk-secret-123"}) + call = run.await_args + assert call is not None + argv = call.args + assert "sk-secret-123" not in "\n".join(argv) + assert "base64 -d" in argv[-1] + + @pytest.mark.asyncio + async def test_exec_rejects_non_main_service(self, make_sandbox): + sandbox = _started(make_sandbox) + with pytest.raises(ValueError, match="single-container"): + await sandbox.exec("true", service="target") + + @pytest.mark.asyncio + async def test_exec_timeout_forces_cleanup(self, make_sandbox): + """Guards PR #936 against retaining a VM after an exec timeout.""" + + sandbox = _started(make_sandbox) + with ( + patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, side_effect=TimeoutError + ), + patch.object(sandbox, "_force_cleanup", new_callable=AsyncMock) as cleanup, + pytest.raises(RuntimeError, match="timed out"), + ): + await sandbox.exec("sleep 999", timeout_sec=5) + cleanup.assert_awaited_once() + + +class TestFileTransfer: + @pytest.mark.asyncio + async def test_mounted_upload_is_host_copy(self, make_sandbox, tmp_path): + """Guards PR #936 by retaining the established /logs fast path.""" + + sandbox = _started(make_sandbox) + source = tmp_path / "source.txt" + source.write_text("data") + with patch.object(apple_mod.shutil, "copy2") as copy: + await sandbox.upload_file(source, "/logs/verifier/out.txt") + assert sandbox.rollout_paths is not None + copy.assert_called_once_with( + source, sandbox.rollout_paths.rollout_dir / "verifier" / "out.txt" + ) + + @pytest.mark.asyncio + async def test_unmounted_upload_uses_native_copy(self, make_sandbox, tmp_path): + """Guards PR #936 against base64 file transfer and host task mutation.""" + + sandbox = _started(make_sandbox) + source = tmp_path / "source.txt" + source.write_text("data") + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.upload_file(source, "/app/out.txt") + calls = [call.args for call in run.await_args_list] + assert calls[-1] == ("cp", str(source), "bf_sess-001:/app/out.txt") + assert all("base64" not in "\n".join(call) for call in calls) + + @pytest.mark.asyncio + async def test_upload_dir_uses_native_copy(self, make_sandbox, tmp_path): + """Guards PR #936 against nesting the host directory under its target.""" + + sandbox = _started(make_sandbox) + source = tmp_path / "source" + source.mkdir() + source.joinpath("file.txt").write_text("data") + source.joinpath("nested").mkdir() + source.joinpath("nested", "child.txt").write_text("nested data") + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.upload_dir(source, "/skills") + copy_calls = [call.args for call in run.await_args_list if call.args[0] == "cp"] + assert copy_calls == [ + ("cp", str(source / "file.txt"), "bf_sess-001:/skills/"), + ("cp", str(source / "nested"), "bf_sess-001:/skills/"), + ] + + @pytest.mark.asyncio + async def test_unmounted_download_uses_native_copy(self, make_sandbox, tmp_path): + """Guards PR #936 against shell-encoded downloads from the VM.""" + + sandbox = _started(make_sandbox) + target = tmp_path / "out.bin" + with patch.object( + apple_mod, "_run_cli", new_callable=AsyncMock, return_value=_success() + ) as run: + await sandbox.download_file("/opt/data.bin", target) + run.assert_awaited_once_with( + "cp", + "bf_sess-001:/opt/data.bin", + str(target), + timeout=120, + ) + + def test_mounted_path_rejects_parent_traversal(self, make_sandbox): + """Guards PR #936 against escaping the rollout directory through /logs.""" + + sandbox = _started(make_sandbox) + with pytest.raises(ValueError, match="Unsafe mounted path"): + sandbox._mounted_host_path("/logs/../../outside") + + +class TestProperties: + def test_backend_properties(self, make_sandbox): + """Guards PR #936 by preserving the common sandbox capability contract.""" + + sandbox = _started(make_sandbox) + assert sandbox.is_mounted is True + assert sandbox.sandbox_id == "bf_sess-001" + assert sandbox.supports_snapshot is False + + +@pytest.mark.skipif( + sys.platform != "darwin" or not shutil.which("container"), + reason="Requires macOS with Apple Container 1.1+", +) +@pytest.mark.asyncio +async def test_real_apple_container_lifecycle_and_copy(tmp_path): + """Guards PR #936 with a real detached lifecycle, exec, and native copy.""" + + environment_dir = tmp_path / "environment" + environment_dir.mkdir() + environment_dir.joinpath("Dockerfile").write_text("FROM ubuntu:24.04\n") + rollout_dir = tmp_path / "rollout" + paths = RolloutPaths(rollout_dir) + config = SandboxConfig( + cpus=1, + memory_mb=512, + docker_image="ubuntu:24.04", + build_timeout_sec=120, + allow_internet=True, + network_mode=NetworkMode.PUBLIC, + ) + + AppleContainerSandbox.preflight() + sandbox = AppleContainerSandbox( + environment_dir=environment_dir, + environment_name="integration-test", + session_id="integration-936", + rollout_paths=paths, + task_env_config=config, + ) + try: + await sandbox.start(force_build=False) + result = await sandbox.exec("printf hello", timeout_sec=10) + assert result == ExecResult(stdout="hello", stderr=None, return_code=0) + + source = tmp_path / "upload.txt" + source.write_text("uploaded content") + await sandbox.upload_file(source, "/tmp/upload.txt") + downloaded = tmp_path / "downloaded.txt" + await sandbox.download_file("/tmp/upload.txt", downloaded) + assert downloaded.read_text() == "uploaded content" + + source_dir = tmp_path / "upload-dir" + source_dir.mkdir() + source_dir.joinpath("nested").mkdir() + source_dir.joinpath("nested", "child.txt").write_text("nested content") + await sandbox.upload_dir(source_dir, "/tmp/dir-target") + result = await sandbox.exec("cat /tmp/dir-target/nested/child.txt") + assert result.stdout == "nested content" + finally: + await sandbox.stop(delete=True) diff --git a/tests/test_litellm_runtime.py b/tests/test_litellm_runtime.py index cc83e5dd7..c47a22c8f 100644 --- a/tests/test_litellm_runtime.py +++ b/tests/test_litellm_runtime.py @@ -158,11 +158,45 @@ async def fake_sandbox_start(**kwargs): ) assert starts[0]["sandbox"] is sandbox + assert provider_runtime is not None assert provider_runtime.base_url == "http://127.0.0.1:45678" assert updated["LLM_BASE_URL"] == "http://127.0.0.1:45678/v1" assert updated["LLM_MODEL"].startswith("openai/benchflow-aws-bedrock") +@pytest.mark.asyncio +async def test_apple_container_uses_sandbox_local_litellm(monkeypatch): + """Guards PR #936 against handing the VM a host-loopback model endpoint.""" + + starts = [] + + async def fake_sandbox_start(**kwargs): + starts.append(kwargs) + return FakeLiteLLMServer("http://127.0.0.1:45678", kwargs["route"]) + + async def unexpected_host_start(**_kwargs): + raise AssertionError("Apple Container must not use a host-local proxy") + + monkeypatch.setattr(runtime_mod, "_start_sandbox_litellm", fake_sandbox_start) + monkeypatch.setattr(runtime_mod, "_start_host_litellm", unexpected_host_start) + sandbox = SimpleNamespace() + + updated, provider_runtime = await ensure_litellm_runtime( + agent="openhands", + agent_env={"OPENAI_API_KEY": "sk-test"}, + model="openai/gpt-4.1-mini", + runtime=None, + environment="apple-container", + session_id="run-936", + sandbox=sandbox, + ) + + assert starts[0]["sandbox"] is sandbox + assert provider_runtime is not None + assert provider_runtime.base_url == "http://127.0.0.1:45678" + assert updated["LLM_BASE_URL"] == "http://127.0.0.1:45678/v1" + + @pytest.mark.asyncio async def test_openhands_registered_provider_can_route_via_explicit_proxy(monkeypatch): """Guards PR #780: OpenHands keeps BenchFlow tracking over explicit proxy env.""" diff --git a/tests/test_oracle_chokepoint.py b/tests/test_oracle_chokepoint.py index ab954b6a8..f15557c2b 100644 --- a/tests/test_oracle_chokepoint.py +++ b/tests/test_oracle_chokepoint.py @@ -68,14 +68,15 @@ def test_bench_eval_create_help_resolves(self): pytest.param(["environment", "create", "--help"], id="environment-create"), ], ) - def test_sandbox_help_matches_v04_supported_backends(self, command): - """Guards ENG-92 CLI help does not advertise future sandbox backends.""" + def test_sandbox_help_matches_registered_backends(self, command): + """Guards ENG-92 CLI help against drifting from the sandbox registry.""" from benchflow.cli.main import app + from benchflow.sandbox.providers import providers_phrase result = CliRunner().invoke(app, command) assert result.exit_code == 0 - assert "Sandbox: docker, daytona, or modal" in result.stdout + assert f"Sandbox: {providers_phrase()}" in result.stdout assert "firecracker" not in result.stdout.lower() assert "kubernetes" not in result.stdout.lower() assert "k8s" not in result.stdout.lower() diff --git a/tests/test_process.py b/tests/test_process.py index 8f26cbf41..244f8d9bc 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -1,5 +1,6 @@ """Tests for process.py env handling (no Docker required).""" +import asyncio import os import shlex import subprocess @@ -8,7 +9,12 @@ import pytest -from benchflow.sandbox.process import DaytonaProcess, DaytonaPtyProcess, DockerProcess +from benchflow.sandbox.process import ( + AppleContainerProcess, + DaytonaProcess, + DaytonaPtyProcess, + DockerProcess, +) class _FakeStdin: @@ -326,6 +332,90 @@ async def capture_communicate(data=None): ) +class TestAppleContainerProcess: + @staticmethod + def _fake_exec(calls, inputs): + async def fake_exec(*args, **kwargs): + calls.append((args, kwargs)) + + async def communicate(data=None): + if data is not None: + inputs.append(data) + return b"", b"" + + return _FakeProcess(communicate=communicate) + + return fake_exec + + def test_from_sandbox_env_requires_started_container(self): + """Guards PR #936 against silently selecting a nonexistent VM.""" + + with pytest.raises(RuntimeError, match="not started"): + AppleContainerProcess.from_sandbox_env(MagicMock(_container_name=None)) + + @pytest.mark.asyncio + async def test_native_exec_is_interactive_and_uses_workdir(self): + """Guards PR #936 by preserving the bidirectional ACP stdio contract.""" + + calls = [] + inputs = [] + with patch( + "benchflow.sandbox.process.asyncio.create_subprocess_exec", + side_effect=self._fake_exec(calls, inputs), + ): + proc = AppleContainerProcess("bf_run") + await proc.start("codex-acp", cwd="/root") + + assert len(calls) == 1 + assert calls[0][0] == ( + "container", + "exec", + "--interactive", + "--workdir", + "/root", + "bf_run", + "bash", + "-c", + "codex-acp", + ) + assert calls[0][1]["stdin"] is asyncio.subprocess.PIPE + assert calls[0][1]["stdout"] is asyncio.subprocess.PIPE + + @pytest.mark.asyncio + async def test_secret_env_uses_stdin_not_process_argv(self): + """Guards PR #936 against exposing provider keys in host process args.""" + + calls = [] + inputs = [] + with patch( + "benchflow.sandbox.process.asyncio.create_subprocess_exec", + side_effect=self._fake_exec(calls, inputs), + ): + proc = AppleContainerProcess("bf_run") + await proc.start( + "codex-acp", + env={"API_KEY": "sk-secret; still-one-value"}, + ) + + assert len(calls) == 2 + assert "sk-secret" not in "\n".join( + str(arg) for call_args, _kwargs in calls for arg in call_args + ) + assert inputs == [b"export API_KEY='sk-secret; still-one-value'\n"] + write_args = calls[0][0] + assert write_args[:5] == ( + "container", + "exec", + "--interactive", + "--user", + "root", + ) + main_command = calls[1][0][-1] + assert ". /tmp/.benchflow_agent_env_" in main_command + assert "rm -f /tmp/.benchflow_agent_env_" in main_command + assert main_command.endswith("&& codex-acp") + + class TestDaytonaProcessEnvFilePath: """Regression: env-file path must be unique without relying on shell `$$` expansion. diff --git a/tests/test_runtime_capabilities.py b/tests/test_runtime_capabilities.py index 17969d9db..4d1ec44f4 100644 --- a/tests/test_runtime_capabilities.py +++ b/tests/test_runtime_capabilities.py @@ -159,7 +159,21 @@ def test_validator_reports_unknown_sandbox_backend() -> None: assert [(issue.path, issue.reason) for issue in issues] == [ ( "sandbox", - "unknown sandbox backend; use docker, daytona, or modal", + "unknown sandbox backend; use docker, daytona, modal, or apple-container", + ) + ] + + +def test_validator_reports_apple_container_no_network_gap() -> None: + """Guards PR #936 against silently launching no-network tasks on Apple Container.""" + config = TaskConfig.model_validate({"environment": {"network_mode": "no-network"}}) + + issues = validate_task_runtime_support(config, sandbox="apple-container") + + assert [(issue.path, issue.reason) for issue in issues] == [ + ( + "environment.network_mode", + "network_mode='no-network' is not enforced by apple-container", ) ] diff --git a/tests/test_sandbox_provider_registry_drift.py b/tests/test_sandbox_provider_registry_drift.py index 1b9c9908a..add1ca0ac 100644 --- a/tests/test_sandbox_provider_registry_drift.py +++ b/tests/test_sandbox_provider_registry_drift.py @@ -1,9 +1,9 @@ """Drift guard for the canonical sandbox-provider registry (dev-ex #14). Before ``benchflow.sandbox.providers`` the set ``{docker, daytona, modal}`` (and -its ``{daytona, modal}`` off-box subset) was hand-copied across ~10 sites with no +its model-proxy placement subset) was hand-copied across ~10 sites with no single source of truth. These tests fail if (a) a literal provider set reappears -outside the registry, (b) the derived facts (phrase, extras, off-box subset) go +outside the registry, (b) the derived facts (phrase, extras, proxy placement) go stale, or (c) the registry and the dispatch table drift apart. """ @@ -16,8 +16,11 @@ from benchflow.sandbox.providers import ( OFF_BOX_MODEL_PROVIDERS, OPTIONAL_SANDBOX_EXTRAS, + PROVIDERS_BY_NAME, + SANDBOX_MODEL_PROXY_PROVIDERS, SANDBOX_PROVIDER_SET, SANDBOX_PROVIDERS, + ModelProxyLocation, providers_phrase, ) @@ -28,21 +31,31 @@ def test_registry_is_the_single_source_of_truth() -> None: # Locks the current set + docker-first order; adding a provider is then a # deliberate edit here + a test update, never a silent scatter. - assert SANDBOX_PROVIDERS == ("docker", "daytona", "modal") + assert SANDBOX_PROVIDERS == ("docker", "daytona", "modal", "apple-container") assert frozenset(SANDBOX_PROVIDERS) == SANDBOX_PROVIDER_SET def test_providers_phrase_is_byte_identical() -> None: # The refactor must be behavior-preserving for every help/error string that # used to hand-write this phrase. - assert providers_phrase() == "docker, daytona, or modal" - assert providers_phrase(quote=True) == "'docker', 'daytona', or 'modal'" + assert providers_phrase() == "docker, daytona, modal, or apple-container" + assert ( + providers_phrase(quote=True) + == "'docker', 'daytona', 'modal', or 'apple-container'" + ) + +def test_model_proxy_placement_is_explicit_for_every_provider() -> None: + """Guards PR #936 against routing host loopback into an Apple VM.""" -def test_off_box_subset_is_exactly_the_non_docker_providers() -> None: - # The two former {daytona, modal} frozensets are now derived; this locks the - # property so a 4th provider can't silently miss the off-box routing. - assert SANDBOX_PROVIDER_SET - {"docker"} == OFF_BOX_MODEL_PROVIDERS + assert PROVIDERS_BY_NAME["docker"].model_proxy is ModelProxyLocation.HOST + for provider in ("daytona", "modal", "apple-container"): + assert PROVIDERS_BY_NAME[provider].model_proxy is ModelProxyLocation.SANDBOX + assert ( + frozenset({"daytona", "modal", "apple-container"}) + == SANDBOX_MODEL_PROXY_PROVIDERS + ) + assert OFF_BOX_MODEL_PROVIDERS is SANDBOX_MODEL_PROXY_PROVIDERS def test_no_divergent_provider_set_literal_outside_the_registry() -> None: @@ -81,8 +94,11 @@ def test_optional_extras_match_pyproject() -> None: f"registry extras {set(OPTIONAL_SANDBOX_EXTRAS.values())} not all declared " f"in pyproject optional-dependencies {declared}" ) - # Every off-box provider needs an extra (docker is built in, needs none). - assert set(OPTIONAL_SANDBOX_EXTRAS) == SANDBOX_PROVIDER_SET - {"docker"} + # Apple Container is a system CLI backend, so it needs no Python extra. + assert OPTIONAL_SANDBOX_EXTRAS == { + "daytona": "sandbox-daytona", + "modal": "sandbox-modal", + } def test_every_registry_provider_has_a_dispatch_branch() -> None: diff --git a/uv.lock b/uv.lock index e55180107..dec20d7f7 100644 --- a/uv.lock +++ b/uv.lock @@ -1982,7 +1982,7 @@ wheels = [ [[package]] name = "mcp" -version = "1.27.2" +version = "1.28.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -2000,9 +2000,9 @@ dependencies = [ { name = "typing-inspection" }, { name = "uvicorn", marker = "sys_platform != 'emscripten'" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/27/3c/347cf965d313f5d41764e7d46bea6ffe7d9ef13b983cc429b0340962a082/mcp-1.27.2.tar.gz", hash = "sha256:8e02db104096d1c25b28e64bde29a5c32b31bc241710213e12fd4d84985bdfef", size = 621116, upload-time = "2026-05-29T17:16:04.039Z" } +sdist = { url = "https://files.pythonhosted.org/packages/6e/77/9450b8f251a13affb6281997d0523c4615f8a8b35d0b21ff30db3a5aac9d/mcp-1.28.1.tar.gz", hash = "sha256:d51e36a5f5644faea4f85ea649bfffa6bc6c26770d42798ad6a3de3d2ba69683", size = 638501, upload-time = "2026-06-26T12:57:29.093Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/c9/11/252c6f971dc4f16af1d98a1c469d8ba523aab00d1bb76b4d3bc1ff32eacc/mcp-1.27.2-py3-none-any.whl", hash = "sha256:d6ff5160c6ca65d93013626efb3fc249de683c30b2d8570755ceddd490344de5", size = 220498, upload-time = "2026-05-29T17:16:02.442Z" }, + { url = "https://files.pythonhosted.org/packages/e2/5e/d118fce19f87a2e7d8101c35c8ae0ec289098a4df0ff244cec23e415aca0/mcp-1.28.1-py3-none-any.whl", hash = "sha256:2726bca5e7193f61c5dde8b12500a6de2d9acf6d1a1c0be9e8c2e706437991df", size = 222620, upload-time = "2026-06-26T12:57:27.218Z" }, ] [[package]]