fix(sandbox): harden local Docker sandbox containers and port binding - #4986
fix(sandbox): harden local Docker sandbox containers and port binding#4986simpleqt wants to merge 4 commits into
Conversation
Root causes (security audit SBX-1/SBX-2) in the local container backend: - _resolve_docker_bind_host published sandbox ports on 0.0.0.0 whenever DEER_FLOW_SANDBOX_HOST was non-loopback (docker-compose defaults to host.docker.internal), exposing the unauthenticated /v1/shell/* exec API on every host interface. - _start_container ran every sandbox with seccomp=unconfined and no capability, privilege-escalation, or resource limits, so untrusted model-authored code could exhaust the host, escalate privileges, and reach internal networks / cloud metadata endpoints directly. Hardening changes and defaults: - Port binding: non-loopback sandbox hosts now bind the Docker default bridge gateway instead of 0.0.0.0, discovered dynamically via `docker network inspect bridge` with a static 172.17.0.1 fallback. host.docker.internal resolves to that gateway through host-gateway, so DooD gateways and the Docker host still reach the sandbox while external interfaces no longer see the port. DEER_FLOW_SANDBOX_BIND_HOST=0.0.0.0 restores the legacy broad bind. - seccomp=unconfined is no longer unconditional: sandboxes run with Docker's default seccomp profile; opt back in with DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=1, only when the sandbox image is verified to require syscalls the default profile blocks. - Add --cap-drop=ALL and --security-opt no-new-privileges (Docker only; the Apple Container CLI does not support these flags). - Bounded resources with env overrides: --memory 2g (DEER_FLOW_SANDBOX_MEMORY), --cpus 2 (DEER_FLOW_SANDBOX_CPUS), --pids-limit 512 (DEER_FLOW_SANDBOX_PIDS_LIMIT); each also accepts "0"/"none" to disable the limit. - No --user is forced by default (the default AIO sandbox image's user is upstream-controlled and unverified), but DEER_FLOW_SANDBOX_CONTAINER_USER passes one through for deployments that know their image. - DEER_FLOW_SANDBOX_NETWORK passes --network so sandboxes can be attached to a dedicated egress-controlled network; default networking is unchanged. backend/docs/CONFIGURATION.md documents the new bind behavior and every override; tests cover each default and escape hatch.
willem-bd
left a comment
There was a problem hiding this comment.
Thanks for hardening this path. I found two runtime compatibility issues that should be addressed before merge: the shipped AIO image requires a Chromium-compatible seccomp policy, and bind selection must follow the actual host-gateway mapping, including configured and IPv6 cases. Focused tests, Ruff, and CI pass, but the current argv-only tests do not cover these runtime failures.
| # unconditionally, disabling syscall filtering for every sandbox; | ||
| # opt back in only when the sandbox image is verified to need | ||
| # syscalls that the default profile blocks. | ||
| if _env_flag_enabled("DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED"): |
There was a problem hiding this comment.
[P1] Preserve a Chromium-compatible seccomp policy
The default image's upstream quick-start always runs it with --security-opt seccomp=unconfined, and the upstream FAQ explicitly says its browser will not start under Docker's default profile because Chromium needs namespace-related syscalls. With this environment variable unset, every DeerFlow Docker AIO sandbox now uses that incompatible profile. Please either ship and select a restricted Chromium-compatible seccomp profile or retain the required option for the shipped default image; an argv-only unit test does not validate image startup or browser functionality.
References: https://github.com/agent-infra/sandbox/blob/0f23e3c9395cd2175f5f8d009c96363f2a7711a5/website/docs/en/guide/start/quick-start.mdx and https://github.com/agent-infra/sandbox/blob/0f23e3c9395cd2175f5f8d009c96363f2a7711a5/website/docs/en/guide/start/faq.md
|
|
||
| logger.debug("Docker sandbox bind: 0.0.0.0 (non-loopback sandbox host compatibility)") | ||
| return "0.0.0.0" | ||
| gateway = _docker_bridge_gateway_ip() or _DOCKER_BRIDGE_GATEWAY_FALLBACK |
There was a problem hiding this comment.
[P2] Use the actual host-gateway mapping
Docker allows host-gateway to be overridden with the daemon's host-gateway-ip setting and it can also resolve to IPv6. In those valid configurations, host.docker.internal inside the Gateway resolves to the configured address, while this code binds the sandbox port to the default bridge IPv4 (or the static 172.17.0.1 fallback). The returned sandbox URL then targets an address where the port is not listening, so readiness and acquisition fail. Please derive the bind address from the actual host-gateway mapping, or plumb the same configured value to both sides, and handle IPv6 formatting rather than falling back to a nonexistent IPv4 address.
Reference: https://docs.docker.com/reference/cli/dockerd/#configure-host-gateway-ip
…red seccomp default Review follow-ups on the hardening change: - Bind: resolve the sandbox host itself and bind that address, instead of assuming the default bridge IPv4. host.docker.internal follows the daemon host-gateway-ip mapping (customizable, possibly IPv6), so the resolved address is exactly where the gateway connects — the published port and advertised URL always match. IPv6 is bracketed for docker -p, zone ids stripped, wildcard resolutions ignored; unresolved hosts fall back to the bridge gateway with a warning pointing at DEER_FLOW_SANDBOX_BIND_HOST. - seccomp: the shipped AIO image needs seccomp=unconfined for its Chromium browser (upstream quick-start always passes it; the upstream FAQ documents the browser failing under Docker default profile), so that option returns as the default. Tightening stays possible via DEER_FLOW_SANDBOX_SECCOMP_PROFILE=<path to a restricted, Chromium-compatible profile> or DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=0 for images verified to work with Docker's default profile. - cap-drop/no-new-privileges and the resource limits are unchanged. - Tests updated for both behaviors; 37 pass.
|
Thanks for the review — both points addressed in 18d9d54: P1 (seccomp): P2 (bind): Tests updated for both (resolve-path v4/v6/zone-id/wildcard, fallback chain, seccomp default/opt-out/profile precedence) — 37 passing. |
willem-bd
left a comment
There was a problem hiding this comment.
Two remaining issues in the latest hardening follow-up: the explicit IPv6 bind escape hatch is not normalized for Docker publish syntax, and the security guide overstates the default seccomp protection.
| 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.", |
There was a problem hiding this comment.
[P2] Normalize explicit IPv6 bind overrides
DEER_FLOW_SANDBOX_BIND_HOST is returned verbatim, so the IPv6 escape hatch recommended by the fallback warning produces an invalid publish spec for a normal value such as fd00::1: _start_container turns it into fd00::1:<port>:8080. Docker requires the host IPv6 literal to be bracketed ([fd00::1]:<port>:8080). Please normalize raw/bracketed IPv6 literals before constructing -p and cover an explicit IPv6 override in the argv tests. Reference: https://docs.docker.com/engine/network/port-publishing/
There was a problem hiding this comment.
Fixed in 2ba733d — the explicit override now goes through _normalize_docker_bind_spec(): raw IPv6 literals get bracketed, already-bracketed ones are canonicalized, IPv4/hostnames pass through untouched. Added test_resolve_docker_bind_host_brackets_bare_ipv6_override (unit) and test_start_container_brackets_bare_ipv6_bind_override (argv-level, asserts -p [fd00::1]:18080:8080). 39/39 backend tests pass.
|
|
||
| The sandbox HTTP API (`/v1/shell/*` and friends) has no authentication: anyone who can reach a published sandbox port can execute arbitrary commands in that sandbox. For bare-metal Docker sandbox runs that use localhost, DeerFlow binds the sandbox port to `127.0.0.1` so it is not exposed on other host interfaces. For Docker-outside-of-Docker deployments that connect through `host.docker.internal`, the port is bound to the address that hostname actually resolves to — the daemon's `host-gateway-ip` mapping (customizable, possibly IPv6) — so the published port and the address the gateway connects to always match, and the port is no longer published on external network interfaces (previously it was bound to `0.0.0.0`). If resolution fails, the Docker default bridge gateway (via `docker network inspect bridge`, falling back to `172.17.0.1`) is used as a best-effort bind and a warning is logged. Set `DEER_FLOW_SANDBOX_BIND_HOST` explicitly if your deployment needs a different bind address; setting it to `0.0.0.0` restores the legacy broad bind, which re-exposes the unauthenticated exec API on every interface and should be paired with an external firewall. | ||
|
|
||
| Local Docker sandbox containers are also hardened by default: all Linux capabilities are dropped (`--cap-drop=ALL`), privilege escalation is blocked (`no-new-privileges`), Docker's default seccomp profile stays active, and resources are bounded. The following environment variables (set them in the gateway process, e.g. via `.env` loaded by docker-compose, or the gateway service `environment:`) tune or disable each knob: |
There was a problem hiding this comment.
[P2] Do not claim the default seccomp profile remains active
This paragraph says Docker's default seccomp profile stays active, but _start_container adds seccomp=unconfined unless the operator explicitly opts out or supplies a custom profile; the table immediately below correctly describes that behavior. Because unconfined disables syscall filtering, this sentence materially overstates the default sandbox protection. Please make the overview match the implementation and table.
There was a problem hiding this comment.
Fixed in 2ba733d — the overview now states the shipped image runs with seccomp=unconfined (syscall filtering disabled) because its Chromium needs it, matching the table and the implementation, and points at the two variables that change it.
… accurately DEER_FLOW_SANDBOX_BIND_HOST was returned verbatim, so a bare IPv6 literal like fd00::1 produced an invalid publish spec (fd00::1:port:8080); Docker requires the bracketed form. Normalize raw and already-bracketed IPv6 literals (IPv4/hostnames untouched), with resolver-level and argv-level tests covering the explicit IPv6 override. The CONFIGURATION.md overview claimed Docker's default seccomp profile stays active, contradicting the seccomp=unconfined default the table (and the code) actually ship for the Chromium-based image; spell out the relaxed default and where to change it.
willem-bd
left a comment
There was a problem hiding this comment.
Re-reviewed current head 5f663a4. The Ruff formatting blocker found on the previous head is fixed by the latest commit, and the two earlier runtime/documentation findings remain addressed. The focused sandbox suite passes (138 tests), and Ruff lint and formatting checks pass. No remaining actionable findings.
|
The earlier lint-backend failure was a ruff format drift on |
|
Both P2s from the review of 18d9d54 landed in 2ba733d (pushed ~45 minutes after that review; the review's P2 — explicit IPv6 bind override not normalized: P2 — security guide overstating the default seccomp protection: the paragraph in (The follow-up 5f663a4 is just the ruff-format pass over the same file; no further behavior change.) |
Root causes (security audit SBX-1/SBX-2) in the local container backend:
DEER_FLOW_SANDBOX_HOST was non-loopback (docker-compose defaults to
host.docker.internal), exposing the unauthenticated /v1/shell/* exec
API on every host interface.
capability, privilege-escalation, or resource limits, so untrusted
model-authored code could exhaust the host, escalate privileges, and
reach internal networks / cloud metadata endpoints directly.
Hardening changes and defaults:
bridge gateway instead of 0.0.0.0, discovered dynamically via
docker network inspect bridgewith a static 172.17.0.1 fallback.host.docker.internal resolves to that gateway through host-gateway,
so DooD gateways and the Docker host still reach the sandbox while
external interfaces no longer see the port.
DEER_FLOW_SANDBOX_BIND_HOST=0.0.0.0 restores the legacy broad bind.
Docker's default seccomp profile; opt back in with
DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=1, only when the sandbox image
is verified to require syscalls the default profile blocks.
the Apple Container CLI does not support these flags).
(DEER_FLOW_SANDBOX_MEMORY), --cpus 2 (DEER_FLOW_SANDBOX_CPUS),
--pids-limit 512 (DEER_FLOW_SANDBOX_PIDS_LIMIT); each also accepts
"0"/"none" to disable the limit.
is upstream-controlled and unverified), but
DEER_FLOW_SANDBOX_CONTAINER_USER passes one through for deployments
that know their image.
attached to a dedicated egress-controlled network; default networking
is unchanged.
backend/docs/CONFIGURATION.md documents the new bind behavior and every
override; tests cover each default and escape hatch.
Security context (from a defensive audit): see body above; every legacy behavior keeps an opt-out env documented in backend/docs/CONFIGURATION.md.