mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-14 16:08:41 +00:00
* fix(sandbox): harden local Docker sandbox containers and port binding 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. * fix(sandbox): follow host-gateway mapping for binds; keep image-required 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. * fix(sandbox): bracket bare IPv6 bind overrides; state seccomp default 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. * style(sandbox): apply ruff format to local_backend * fix(sandbox): reject host networking, force builtin seccomp opt-out, resolve hostname binds Review follow-up on #4986 (willem-bd): - P1: DEER_FLOW_SANDBOX_NETWORK=host (and container:<name>) now raise a RuntimeError at start instead of silently voiding the hardened port bind — Docker discards -p/--publish in host mode and shares the network namespace for container:<name>, which would re-expose the unauthenticated exec API on the host's interfaces. Two regression tests cover both rejections. - P2: the seccomp opt-out now passes seccomp=builtin explicitly instead of omitting the option, so a daemon configured with an unconfined or custom default cannot weaken the documented opt-out; the test asserts the flag. - P2: hostname values in DEER_FLOW_SANDBOX_BIND_HOST resolve to an address before use (Docker publish specs require an IP literal as the host part, so host.docker.internal previously produced an invalid spec that prevented every sandbox from starting); unresolvable names raise a clear configuration error. Tests cover resolution and rejection; CONFIGURATION.md updated for all three behaviors. 43/43 pass in tests/test_aio_sandbox_local_backend.py; ruff check + format clean. * fix(sandbox): reject DEER_FLOW_SANDBOX_NETWORK=none (loopback-only, breaks published API port) * fix(sandbox): validate the effective Docker network target; normalize IPv6 sandbox hosts once name=host / name=none dodge raw-string checks but attach like the bare words; strip name= prefixes and validate the effective target (network IDs keep passing). Bracketed IPv6 sandbox hosts now resolve for the bind and bare IPv6 hosts produce bracketed URL authorities — both input forms give identical bind and URL addresses. * fix(sandbox): parse the full Docker network long syntax before validating Docker accepts comma-separated key=value fields in any order (name=, gw-priority=, alias=, ...); a name=host field hides the host network behind surrounding fields. Parse the CSV and validate the parsed name= target (last occurrence wins, fields lowercased, mirroring opts/network.go); no-name values fall through like Docker's own rejection. * fix(sandbox): keep CHOWN/SETUID/SETGID through cap-drop=ALL for the default image The shipped image's entrypoint starts as root, creates the gem user, chowns /opt/jupyter and drops to that user via su; without those three capabilities the set -e script dies before the readiness endpoint exists. no-new-privileges stays (it blocks gaining privileges via exec, not using the added caps). Adds a docker-gated real-image startup smoke test. * fix(sandbox): let pre-initialized non-root images drop the startup capabilities The CHOWN/SETUID/SETGID re-add only exists for the shipped image's root entrypoint handoff. A custom image that never runs as root gets an explicit opt-out (DEER_FLOW_SANDBOX_IMAGE_STARTUP_CAPS=0) so those capabilities are not left available to sandboxed code (chown on bind mounts, UID/GID impersonation). * test(sandbox): gate the real-image smoke test behind the live marker The default offline suite (make test = -m 'not live') must not depend on a third-party registry: mark the smoke test live, probe the daemon inside the test body (never at collection time), and allow pinning the image reference via DEER_FLOW_SANDBOX_SMOKE_IMAGE for a dedicated integration job. * test/docs: isolate DEER_FLOW_SANDBOX_IMAGE_STARTUP_CAPS in tests; add table row; split custom-image guidance _clear_hardening_env now clears the new knob so a developer shell or .env preset cannot flip the default-path tests. CONFIGURATION.md gains the table row, and the custom-image guidance becomes its own paragraph with the no-new-privileges scope stated correctly (it does not mitigate the retained CAP_SETUID/SETGID risk). * test(sandbox): make the live smoke test diagnosable 300s readiness budget (cold pull + cold start must not be conflated with broken capabilities) and dump the container's last 40 log lines on failure so the next live run tells us whether the capability set is incomplete (chown/useradd/su errors) or the services are merely slow. * test(ci): align the smoke test with the 60s provider deadline; add a dedicated live smoke workflow Single-source the readiness deadline as SANDBOX_LOCAL_PROVIDER_READY_TIMEOUT (used by both provider paths and the smoke test) so the validation cannot drift from the production contract again. New sandbox-image-smoke.yml runs the live test on a dedicated job, with the image reference pinnable via the SANDBOX_SMOKE_IMAGE repository variable (digest resolved and recorded in the job summary when falling back to :latest). * test(sandbox): pull the failing program's own logs on smoke failure supervisord only surfaces exit codes in docker logs; nginx's stderr lands in files inside the container. Dump supervisor program logs, nginx -t, and the nginx error log on failure so the next run names the exact broken line. * ci(sandbox): export an immutable repo@digest reference for the smoke run docker pull once on the runner platform, resolve RepoDigests[0], and pass that immutable reference to the test via GITHUB_ENV — the recorded and executed images can no longer diverge when the tag moves, and platform selection is left to the daemon instead of jq over the manifest index. * fix(sandbox): add DAC_OVERRIDE — the root nginx master writes gem-owned logs The image's root nginx master opens /var/log/nginx/{access,error}.log, which belong to the gem user, for the container's lifetime; without CAP_DAC_OVERRIDE it dies with 'open() failed (13: Permission denied)' on every start (FATAL under supervisord) and readiness never arrives. Four capabilities now: CHOWN/SETUID/SETGID for the entrypoint handoff plus this runtime log-write need.