fix(sandbox): harden local Docker sandbox containers and port binding #4986
+630
−18
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,12 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import ipaddress | ||
| import json | ||
| import logging | ||
| import os | ||
| import shlex | ||
| import socket | ||
| import subprocess | ||
| from datetime import datetime | ||
|
|
||
|
|
@@ -139,20 +141,105 @@ def _is_loopback_sandbox_host(host: str) -> bool: | |
| return _normalize_sandbox_host(host) in {"", "localhost", "127.0.0.1", "::1", "[::1]"} | ||
|
|
||
|
|
||
| def _normalize_docker_bind_spec(value: str) -> str: | ||
| """Bracket bare IPv6 literals for Docker's ``-p`` publish syntax. | ||
|
|
||
| Docker requires the host part of a publish spec to be a bracketed IPv6 | ||
| literal (``[fd00::1]:port:8080``), but operators writing the bind override | ||
| naturally give the bare address. Raw and already-bracketed IPv6 forms are | ||
| normalized; IPv4 addresses and hostnames pass through unchanged. | ||
| """ | ||
| candidate = value.strip() | ||
| inner = candidate | ||
| if candidate.startswith("[") and candidate.endswith("]"): | ||
| inner = candidate[1:-1] | ||
| try: | ||
| if ipaddress.ip_address(inner).version == 6: | ||
| return f"[{inner}]" | ||
| except ValueError: | ||
| pass | ||
| return candidate | ||
|
|
||
|
|
||
| # Fallback gateway of Docker's default bridge network (docker0). Used when the | ||
| # daemon cannot be queried (see _docker_bridge_gateway_ip) so non-loopback | ||
| # sandbox deployments still get a host-only bind instead of 0.0.0.0. | ||
| _DOCKER_BRIDGE_GATEWAY_FALLBACK = "172.17.0.1" | ||
|
|
||
| # Hardening defaults for sandbox containers. The sandbox executes untrusted, | ||
| # model-authored code, so containers get bounded resources by default; every | ||
| # value can be tuned or disabled through the corresponding DEER_FLOW_SANDBOX_* | ||
| # environment variable (see _start_container). | ||
| _DEFAULT_SANDBOX_MEMORY = "2g" | ||
| _DEFAULT_SANDBOX_CPUS = "2" | ||
| _DEFAULT_SANDBOX_PIDS_LIMIT = "512" | ||
|
|
||
|
|
||
| def _docker_bridge_gateway_ip() -> str | None: | ||
| """Return the gateway IPv4 of Docker's default bridge network, or None. | ||
|
|
||
| The gateway is discovered from the daemon (``docker network inspect | ||
| bridge``) because the address is deployment-specific: daemons with a | ||
| custom ``bip`` or rootless/multi-network setups do not use 172.17.0.1. | ||
| Any failure (docker missing, daemon down, unparsable or non-IPv4 output) | ||
| returns None so the caller can fall back to the well-known default. | ||
| """ | ||
| try: | ||
| result = subprocess.run( | ||
| [ | ||
| "docker", | ||
| "network", | ||
| "inspect", | ||
| "bridge", | ||
| "--format", | ||
| "{{(index .IPAM.Config 0).Gateway}}", | ||
| ], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=10, | ||
| ) | ||
| except (OSError, subprocess.TimeoutExpired) as e: | ||
| logger.debug(f"Could not query Docker bridge gateway: {e}") | ||
| return None | ||
| if result.returncode != 0: | ||
| logger.debug(f"docker network inspect bridge failed: {(result.stderr or '').strip()}") | ||
| return None | ||
| candidate = (result.stdout or "").strip() | ||
| try: | ||
| if ipaddress.ip_address(candidate).version != 4: | ||
| return None | ||
| except ValueError: | ||
| return None | ||
| return candidate | ||
|
|
||
|
|
||
| def _resolve_docker_bind_host(sandbox_host: str | None = None, bind_host: str | None = None) -> str: | ||
| """Choose the host interface for legacy Docker ``-p`` sandbox publishing. | ||
|
|
||
| Bare-metal/local runs talk to sandboxes through localhost and bind to | ||
| 127.0.0.1, so the sandbox HTTP API (which has no authentication — anyone | ||
| who can reach it gets arbitrary shell execution) is never exposed on | ||
| other host interfaces. | ||
|
|
||
| Non-loopback sandbox hosts (typically Docker-outside-of-Docker via | ||
| ``host.docker.internal``) used to bind 0.0.0.0, which published the | ||
| unauthenticated exec API on every interface of the host. They now bind | ||
| the address the sandbox host itself resolves to: ``host.docker.internal`` | ||
| follows the daemon's ``host-gateway-ip`` mapping (customizable, possibly | ||
| IPv6), so resolving it yields exactly where the gateway will connect — | ||
| the published port and the advertised sandbox URL always match. Only | ||
| when resolution fails does the default bridge gateway serve as a | ||
| best-effort fallback (with a warning). Operators that genuinely need the | ||
| old broad bind (e.g. remote clients connecting to the sandbox API | ||
| directly) can restore it with ``DEER_FLOW_SANDBOX_BIND_HOST=0.0.0.0`` — | ||
| that re-exposes an unauthenticated shell endpoint and should be paired | ||
| with an external firewall. When operators choose an IPv6 loopback | ||
| sandbox host, bind Docker to IPv6 loopback as well so the advertised | ||
| sandbox URL and published socket use the same address family. | ||
| """ | ||
| explicit_bind = bind_host if bind_host is not None else os.environ.get("DEER_FLOW_SANDBOX_BIND_HOST") | ||
| if explicit_bind is not None: | ||
| explicit_bind = _normalize_docker_bind_spec(explicit_bind) | ||
| if explicit_bind: | ||
| logger.debug("Docker sandbox bind: %s (explicit bind host override)", explicit_bind) | ||
| return explicit_bind | ||
|
|
@@ -165,8 +252,86 @@ def _resolve_docker_bind_host(sandbox_host: str | None = None, bind_host: str | | |
| logger.debug("Docker sandbox bind: 127.0.0.1 (loopback default)") | ||
| return "127.0.0.1" | ||
|
|
||
| resolved = _resolve_sandbox_host_address(host) | ||
| if resolved: | ||
| logger.debug( | ||
| "Docker sandbox bind: %s (resolved from sandbox host %r, follows the daemon host-gateway mapping)", | ||
| resolved, | ||
| host, | ||
| ) | ||
| return resolved | ||
|
|
||
| # Resolution failed (unusual — e.g. a custom hostname with no DNS entry | ||
| # yet). Fall back to the default bridge gateway so non-loopback setups | ||
| # still get a host-only bind, and tell the operator to set the explicit | ||
| # override when their host-gateway-ip is customized or IPv6. | ||
| gateway = _docker_bridge_gateway_ip() or _DOCKER_BRIDGE_GATEWAY_FALLBACK | ||
|
Contributor
There was a problem hiding this comment. [P2] Use the actual host-gateway mapping Docker allows Reference: https://docs.docker.com/reference/cli/dockerd/#configure-host-gateway-ip |
||
| logger.warning( | ||
| "Could not resolve sandbox host %r for the Docker bind; falling back to the default bridge gateway %s. If the daemon's host-gateway-ip is customized or IPv6, set DEER_FLOW_SANDBOX_BIND_HOST to that address explicitly.", | ||
| host, | ||
| gateway, | ||
| ) | ||
| return gateway | ||
|
|
||
|
|
||
| def _env_flag_enabled(name: str) -> bool: | ||
| """Return True when environment variable ``name`` holds an affirmative value.""" | ||
| return os.environ.get(name, "").strip().lower() in {"1", "true", "yes", "on"} | ||
|
|
||
|
|
||
| def _env_flag_disabled(name: str) -> bool: | ||
| """Return True when ``name`` is explicitly set to a negative value. | ||
|
|
||
| For flags whose behavior defaults to ON, only an explicit opt-out | ||
| (``0``/``false``/``no``/``off``) counts as disabled; any other value, | ||
| including unset, keeps the default. | ||
| """ | ||
| return os.environ.get(name, "").strip().lower() in {"0", "false", "no", "off"} | ||
|
|
||
|
|
||
| def _resolve_sandbox_host_address(host: str) -> str | None: | ||
| """Resolve ``host`` to the bind spec Docker should publish sandboxes on. | ||
|
|
||
| ``host.docker.internal`` resolves to whatever the daemon's | ||
| ``host-gateway-ip`` maps it to (customizable and possibly IPv6), so the | ||
| address the gateway will actually *connect* to is exactly this | ||
| resolution — binding it keeps the published port and the advertised | ||
| sandbox URL on the same address instead of guessing the default bridge | ||
| IPv4. IPv6 results are bracketed for Docker's ``-p`` syntax. Returns | ||
| None when the name cannot be resolved. | ||
| """ | ||
| try: | ||
| infos = socket.getaddrinfo(host, None) | ||
| except OSError as e: | ||
| logger.debug(f"Could not resolve sandbox host {host!r}: {e}") | ||
| return None | ||
| for family, _, _, _, sockaddr in infos: | ||
| ip = sockaddr[0] | ||
| if family == socket.AF_INET6: | ||
| # Drop any zone id (%eth0) — Docker bind specs do not accept it. | ||
| ip = ip.split("%", 1)[0] | ||
| if ip in ("::",): | ||
| continue | ||
| return f"[{ip}]" | ||
| if family == socket.AF_INET and ip not in ("0.0.0.0",): | ||
| return ip | ||
| return None | ||
|
|
||
|
|
||
| def _docker_resource_limit(env_name: str, default: str) -> str | None: | ||
| """Resolve a Docker resource limit from the environment with a safe default. | ||
|
|
||
| Unset/empty keeps the secure default; ``0`` or ``none`` disables the limit | ||
| entirely (escape hatch for hosts where the default breaks a workload); | ||
| any other value is passed through verbatim so operators can tune it. | ||
| """ | ||
| raw = os.environ.get(env_name) | ||
| if raw is None or not raw.strip(): | ||
| return default | ||
| value = raw.strip() | ||
| if value.lower() in {"0", "none"}: | ||
| return None | ||
| return value | ||
|
|
||
|
|
||
| def _is_no_such_container_error(stderr: str, container_name: str) -> bool: | ||
|
|
@@ -552,9 +717,59 @@ def _start_container( | |
| """ | ||
| cmd = [self._runtime, "run"] | ||
|
|
||
| # Docker-only security hardening. The sandbox container executes | ||
| # untrusted, model-authored code, so it must not run with the | ||
| # daemon's permissive defaults: all Linux capabilities are dropped, | ||
| # privilege escalation (setuid/sudo) is blocked, and CPU/memory/PID | ||
| # footprints are bounded so one runaway sandbox cannot exhaust the | ||
| # host or fork-bomb it. Each knob has an env escape hatch documented | ||
| # in backend/docs/CONFIGURATION.md. Apple Container's CLI does not | ||
| # support these flags, so they are Docker-only. | ||
| if self._runtime == "docker": | ||
| cmd.extend(["--cap-drop=ALL", "--security-opt", "no-new-privileges"]) | ||
|
|
||
| # The shipped AIO image runs a Chromium-based browser that does | ||
| # not start under Docker's default seccomp profile — its upstream | ||
| # quick-start always passes seccomp=unconfined and the upstream | ||
| # FAQ documents the browser failing under the default profile | ||
| # (Chromium needs namespace-related syscalls). Keep that option | ||
| # as the default so the shipped image keeps working. Two ways to | ||
| # tighten it for a known image: | ||
| # DEER_FLOW_SANDBOX_SECCOMP_PROFILE=/path/to/profile.json | ||
| # → use a restricted, Chromium-compatible profile instead | ||
| # (Docker's default profile plus the needed syscalls); | ||
| # DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=0 | ||
| # → fall back to Docker's default profile, only for images | ||
| # verified to start and pass their browser checks with it. | ||
| seccomp_profile = os.environ.get("DEER_FLOW_SANDBOX_SECCOMP_PROFILE", "").strip() | ||
| if seccomp_profile: | ||
| cmd.extend(["--security-opt", f"seccomp={seccomp_profile}"]) | ||
| elif not _env_flag_disabled("DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED"): | ||
|
Contributor
There was a problem hiding this comment. [P2] Force the built-in seccomp profile for the opt-out When |
||
| cmd.extend(["--security-opt", "seccomp=unconfined"]) | ||
|
|
||
| if memory := _docker_resource_limit("DEER_FLOW_SANDBOX_MEMORY", _DEFAULT_SANDBOX_MEMORY): | ||
| cmd.extend(["--memory", memory]) | ||
| if cpus := _docker_resource_limit("DEER_FLOW_SANDBOX_CPUS", _DEFAULT_SANDBOX_CPUS): | ||
| cmd.extend(["--cpus", cpus]) | ||
| if pids_limit := _docker_resource_limit("DEER_FLOW_SANDBOX_PIDS_LIMIT", _DEFAULT_SANDBOX_PIDS_LIMIT): | ||
| cmd.extend(["--pids-limit", pids_limit]) | ||
|
|
||
| # No --user is forced by default: the default AIO sandbox image | ||
| # is upstream-built and its runtime user is not pinned here, and | ||
| # a wrong user would break the sandbox server's home-directory | ||
| # assumptions. Deployments that know their image's user (and the | ||
| # UID/GID ownership of its mounts) can pass it through. | ||
| if container_user := os.environ.get("DEER_FLOW_SANDBOX_CONTAINER_USER", "").strip(): | ||
| cmd.extend(["--user", container_user]) | ||
|
|
||
| # Default: the daemon's default network (unchanged behavior). | ||
| # Point this at a dedicated, egress-controlled Docker network so | ||
| # sandbox traffic can be filtered by that network's policy — | ||
| # otherwise sandbox code can reach internal networks and cloud | ||
| # metadata endpoints directly, bypassing the gateway's SSRF | ||
| # protections. | ||
| if network := os.environ.get("DEER_FLOW_SANDBOX_NETWORK", "").strip(): | ||
|
Contributor
There was a problem hiding this comment. [P1] Reject host networking before starting the sandbox This unrestricted pass-through accepts |
||
| cmd.extend(["--network", network]) | ||
|
|
||
| if self._runtime == "docker": | ||
| port_mapping = f"{_resolve_docker_bind_host()}:{port}:8080" | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Resolve or reject hostname bind overrides
This helper deliberately passes hostnames through (and the new test blesses
host.docker.internal), but its result becomes the first field of-p, whose full form requiresHOST_IP:HOST_PORT:CONTAINER_PORT. An override such asDEER_FLOW_SANDBOX_BIND_HOST=host.docker.internaltherefore produceshost.docker.internal:18080:8080, which Docker rejects as an invalid host IP and prevents every sandbox from starting. Please resolve the override to an IP literal, or reject non-IP values with a clear configuration error and update the test/docs accordingly. Reference: https://docs.docker.com/get-started/docker-concepts/running-containers/publishing-ports/#publishing-to-a-specific-host-ip