From b9b74d233848db9731709bcecb9de0acb19f8d75 Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Sun, 11 Oct 2026 11:48:31 +0900 Subject: [PATCH] fix(scripts): reject local BIND_HOST values nginx would misread (#6635) * fix(scripts): reject local BIND_HOST values nginx would misread nginx-local-conf.sh checked only for address and hostname characters. BIND_HOST=0 rendered `listen 0:2026;`, which nginx resolves to 0.0.0.0 and binds on every IPv4 interface, and 1.2.3 bound 1.2.0.3. Values nginx rejects outright ([[fd00::1]], -foo) passed the serve.sh preflight and failed only after stop_all had torn down the running stack. Accept exactly a dotted-quad IPv4 address, an IPv6 address (brackets allowed around IPv6 only), or an RFC 1123 hostname. Returning to the loopback default removes a temp/nginx.local.conf rendered for an earlier value, and stop_all now says why it can quit a rendered-config nginx through the tracked config; a test pins the shared pid directive. * docs(changelog): reference #6635 * docs(changelog): match the zh reference spacing * fix(scripts): check every BIND_HOST hostname character before splitting labels read -r -a stops at the first newline, so is_hostname validated only the first line of a value like $'localhost\nbad' and the rest reached the rendered listen directive. Check the allowed characters across the whole value first, and cover multi-line, CR and tab values for each validator. --- .env.example | 1 + CHANGELOG.md | 12 +++ CHANGELOG_zh.md | 10 +++ backend/tests/test_local_dev_loopback_bind.py | 82 ++++++++++++++++++- scripts/AGENTS.md | 6 +- scripts/nginx-local-conf.sh | 77 +++++++++++++++-- scripts/serve.sh | 3 + 7 files changed, 179 insertions(+), 12 deletions(-) diff --git a/.env.example b/.env.example index f00140cf5d..9a0ed23cbc 100644 --- a/.env.example +++ b/.env.example @@ -37,6 +37,7 @@ UNBROWSE_API_KEY=your-unbrowse-api-key # agent can execute commands. # Set 0.0.0.0 only when the host is protected by your own TLS/auth front door # or firewall, and complete first-run setup before it becomes reachable. +# Local runs accept an IPv4 or IPv6 address or a hostname. # BIND_HOST=0.0.0.0 # PORT=2026 diff --git a/CHANGELOG.md b/CHANGELOG.md index a2f2ecf6d8..51291d3c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4388,6 +4388,17 @@ This release closes that milestone with **439 merged pull requests**. `BIND_HOST` (e.g. `BIND_HOST=0.0.0.0` in `.env`) and the `2026` entry; the Gateway and frontend ports are no longer reachable from other machines. +- **scripts:** Reject local `BIND_HOST` values that nginx would misread. The + local nginx helper checked only for address and hostname characters, so + `BIND_HOST=0` rendered `listen 0:2026;`, which nginx resolves to `0.0.0.0` + and binds on every IPv4 interface, and `1.2.3` bound `1.2.0.3`. Values nginx + rejects outright, such as `[[fd00::1]]` or `-foo`, passed the `serve.sh` + preflight and failed only after the running stack had been stopped. The + helper now accepts exactly a dotted-quad IPv4 address, an IPv6 address + (bracketed or not), or an RFC 1123 hostname. Returning to the loopback + default also removes the `temp/nginx.local.conf` rendered for an earlier + value. ([#6635]) + - **skills:** Stop `review_skill_package` from reading other users' skills. Local path targets were allowed anywhere under the Gateway cwd or `/tmp`, and every documented deployment keeps `DEER_FLOW_HOME` under the cwd, so a @@ -9553,3 +9564,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6595]: https://github.com/bytedance/deer-flow/pull/6595 [#6611]: https://github.com/bytedance/deer-flow/pull/6611 [#6613]: https://github.com/bytedance/deer-flow/pull/6613 +[#6635]: https://github.com/bytedance/deer-flow/pull/6635 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index 4190b85024..97a66fc5ef 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -3380,6 +3380,15 @@ 中设置 `BIND_HOST=0.0.0.0`）并使用 `2026` 入口；Gateway 和前端端口不再对其他 机器开放。 +- **脚本：** 本地 `BIND_HOST` 不再接受会被 nginx 误读的值。此前本地 nginx + 辅助脚本只检查字符是否属于地址或主机名，因此 `BIND_HOST=0` 会生成 + `listen 0:2026;`，nginx 将其解析为 `0.0.0.0` 并监听所有 IPv4 网卡，`1.2.3` + 则会绑定到 `1.2.0.3`。`[[fd00::1]]`、`-foo` 等 nginx 直接拒绝的值能通过 + `serve.sh` 的预检，要等正在运行的服务被停止之后才失败。现在该脚本只接受 + 点分十进制 IPv4 地址、IPv6 地址（带或不带方括号）或符合 RFC 1123 的主机名。 + 改回默认的回环地址时，也会删除之前为其他值生成的 + `temp/nginx.local.conf`。([#6635]) + - **技能：** `review_skill_package` 不再能读取其他用户的技能。此前本地路径 目标只要位于 Gateway 工作目录或 `/tmp` 之下即被放行，而所有文档化的部署都把 `DEER_FLOW_HOME` 放在工作目录之下，因此知道他人用户 id 的用户可以传入 @@ -7680,3 +7689,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#6595]: https://github.com/bytedance/deer-flow/pull/6595 [#6611]: https://github.com/bytedance/deer-flow/pull/6611 [#6613]: https://github.com/bytedance/deer-flow/pull/6613 +[#6635]: https://github.com/bytedance/deer-flow/pull/6635 diff --git a/backend/tests/test_local_dev_loopback_bind.py b/backend/tests/test_local_dev_loopback_bind.py index 2c4da09002..398f63b714 100644 --- a/backend/tests/test_local_dev_loopback_bind.py +++ b/backend/tests/test_local_dev_loopback_bind.py @@ -83,6 +83,20 @@ def test_serve_resolves_nginx_config_before_stopping_services(): assert serve.index("scripts/nginx-local-conf.sh") < serve.index('if [ "$ACTION" = "restart" ]; then') +def test_stop_signals_nginx_through_the_tracked_config(): + """``stop_all`` quits nginx via the tracked config even when it runs the rendered copy. + + ``nginx -s quit`` finds the master through the config's ``pid`` directive, so + this works because the render keeps that directive (pinned in + ``test_bind_host_renders_a_config_listening_there``). The tracked path stays + usable for ``--stop`` with an invalid ``BIND_HOST``, which never renders. + """ + serve = SERVE_SH.read_text(encoding="utf-8") + stop_all = serve[serve.index("stop_all() {") : serve.index("\n}\n", serve.index("stop_all() {"))] + + assert 'nginx -c "$REPO_ROOT/docker/nginx/nginx.local.conf" -p "$REPO_ROOT" -s quit' in stop_all + + # ── nginx-local-conf.sh ───────────────────────────────────────────────────── @@ -118,6 +132,19 @@ def test_default_bind_host_uses_the_tracked_config(checkout: Path, bind_host: st assert not (checkout / "temp").exists() +@pytest.mark.parametrize("bind_host", [None, "", "127.0.0.1"], ids=["unset", "blank", "loopback"]) +def test_default_bind_host_removes_a_config_rendered_for_an_earlier_bind_host(checkout: Path, bind_host: str | None): + assert _run(checkout, "0.0.0.0").returncode == 0 + rendered = checkout / "temp" / "nginx.local.conf" + assert rendered.exists() + + result = _run(checkout, bind_host) + + assert result.returncode == 0, result.stderr + assert Path(result.stdout.strip()) == (checkout / "docker" / "nginx" / "nginx.local.conf").resolve() + assert not rendered.exists(), "a stale render with the old listen lines should not be left behind" + + @pytest.mark.parametrize( ("bind_host", "expected"), [ @@ -125,6 +152,11 @@ def test_default_bind_host_uses_the_tracked_config(checkout: Path, bind_host: st ("192.0.2.10", ["192.0.2.10:2026"]), ("fd00::1", ["[fd00::1]:2026"]), ("[fd00::1]", ["[fd00::1]:2026"]), + ("::", ["[::]:2026"]), + ("::ffff:192.0.2.10", ["[::ffff:192.0.2.10]:2026"]), + ("2001:db8:0:0:0:0:0:1", ["[2001:db8:0:0:0:0:0:1]:2026"]), + ("localhost", ["localhost:2026"]), + ("devbox.example-corp.internal", ["devbox.example-corp.internal:2026"]), ], ) def test_bind_host_renders_a_config_listening_there(checkout: Path, bind_host: str, expected: list[str]): @@ -141,9 +173,57 @@ def test_bind_host_renders_a_config_listening_there(checkout: Path, bind_host: s return [line for line in text.splitlines() if not line.strip().startswith("listen ")] assert without_listen(content) == without_listen(NGINX_LOCAL_CONF.read_text(encoding="utf-8")) + # serve.sh's stop_all finds the running master through the tracked config's pid file. + pid_directive = re.compile(r"^\s*pid\s+[^;]+;", re.M) + assert pid_directive.findall(content) == pid_directive.findall(NGINX_LOCAL_CONF.read_text(encoding="utf-8")) != [] -@pytest.mark.parametrize("bind_host", ["0.0.0.0; return 200", "a b", "$(id)"]) +@pytest.mark.parametrize( + "bind_host", + [ + "0.0.0.0; return 200", + "a b", + "$(id)", + # nginx accepts these and binds an address the operator did not write: + # "0" resolves to 0.0.0.0, "1.2.3" to 1.2.0.3, "010.0.0.1" to 10.0.0.1. + "0", + "1.2.3", + "010.0.0.1", + # nginx fails on these at startup ("host not found", "invalid IPv6 + # address"), after serve.sh has already stopped the running stack. + "256.1.1.1", + "1.2.3.4.5", + "-foo", + "foo-", + "foo..bar", + "foo.", + "host_name", + "a" * 64, + "[[fd00::1]]", + "[fd00::1", + "fd00::1]", + "[192.0.2.10]", + "[localhost]", + "fd00:::1", + "fd00::1::2", + "1:2:3:4:5:6:7", + "1:2:3:4:5:6:7:8:9", + "12345::1", + "fd00::1%eth0", + "::ffff:192.0.2", + ":1", + "1:", + # Every validator must see the whole value, not just its first line: + # a second line would become extra listen parameters. + "localhost\nbad", + "localhost\n", + "localhost\r", + "local\thost", + "192.0.2.10\nbad", + "fd00::1\nbad", + "[fd00::1]\nbad", + ], +) def test_bind_host_rejects_values_that_are_not_addresses(checkout: Path, bind_host: str): result = _run(checkout, bind_host) diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index 7501855b99..57fb015b81 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -71,9 +71,9 @@ note rather than exiting early on Apple Container success. The root `PORT` value configures Docker's published nginx ingress only; local orchestration pins Next.js to `3000`. Local runs bind the Gateway and Next.js to `127.0.0.1`; `nginx-local-conf.sh` keeps nginx on loopback or, for a set -`BIND_HOST`, renders `temp/nginx.local.conf` listening there. `serve.sh` -resolves it before stopping anything, so a bad value cannot tear down a running -stack. Keep `dev.mjs`'s all-interfaces default: the Docker dev frontend needs +`BIND_HOST` (strict IP/hostname check), renders `temp/nginx.local.conf` +listening there. `serve.sh` resolves it before stopping anything, so a bad +value cannot tear down a running stack. Keep `dev.mjs`'s all-interfaces default: the Docker dev frontend needs it. Runtime commands launch from the already synchronized environment with `uv run --no-sync`. Production Compose probes Gateway `/health`, and `deploy.sh` waits for all services before reporting diff --git a/scripts/nginx-local-conf.sh b/scripts/nginx-local-conf.sh index 197255f118..1925205bee 100755 --- a/scripts/nginx-local-conf.sh +++ b/scripts/nginx-local-conf.sh @@ -18,19 +18,80 @@ RENDERED_CONF="$REPO_ROOT/temp/nginx.local.conf" bind_host="${BIND_HOST:-127.0.0.1}" if [ "$bind_host" = "127.0.0.1" ]; then + # Drop a copy rendered for an earlier BIND_HOST so its listen lines do not linger. + rm -f "$RENDERED_CONF" printf '%s\n' "$SOURCE_CONF" exit 0 fi -# The value is written into an nginx directive, so accept address and hostname -# characters only. -bind_host="${bind_host#[}" -bind_host="${bind_host%]}" +# Dotted-quad IPv4, every octet 0-255 without leading zeros. nginx resolves +# shorthand forms itself ("0" binds 0.0.0.0, "1.2.3" binds 1.2.0.3). +is_ipv4() { + local IFS=. octet + local -a octets + case "$1" in ''|*[!0-9.]*|.*|*.|*..*) return 1 ;; esac + read -r -a octets <<< "$1" + [ "${#octets[@]}" -eq 4 ] || return 1 + for octet in "${octets[@]}"; do + case "$octet" in 0|[1-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) ;; *) return 1 ;; esac + done +} + +# IPv6 literal: up to eight 1-4 digit hex groups, at most one "::", and an +# optional trailing dotted-quad that counts as two groups. No zone index: +# nginx rejects "%eth0" in listen. +is_ipv6() { + local addr="$1" groups=0 group rest + case "$addr" in *:*) ;; *) return 1 ;; esac + case "$addr" in *[!0-9A-Fa-f:.]*|*:::*|*::*::*) return 1 ;; esac + case "$addr" in :[!:]*|*[!:]:) return 1 ;; esac + if [ "${addr%.*}" != "$addr" ]; then + is_ipv4 "${addr##*:}" || return 1 + addr="${addr%:*}:0:0" + fi + rest="$addr:" + while [ -n "$rest" ]; do + group="${rest%%:*}" + rest="${rest#*:}" + case "$group" in + '') ;; + ?|??|???|????) case "$group" in *[!0-9A-Fa-f]*) return 1 ;; esac; groups=$((groups + 1)) ;; + *) return 1 ;; + esac + done + case "$addr" in + *::*) [ "$groups" -le 7 ] ;; + *) [ "$groups" -eq 8 ] ;; + esac +} + +# RFC 1123 hostname: dot-separated labels of letters, digits and inner hyphens, +# 1-63 characters each, 253 in all. All-numeric values are IPv4 or nothing. +# The character check covers the whole value: read below stops at a newline. +is_hostname() { + local IFS=. label + local -a labels + [ "${#1}" -le 253 ] || return 1 + case "$1" in ''|*[!A-Za-z0-9.-]*|.*|*.|*..*) return 1 ;; esac + case "$1" in *[!0-9.]*) ;; *) return 1 ;; esac + read -r -a labels <<< "$1" + for label in "${labels[@]}"; do + [ "${#label}" -le 63 ] || return 1 + case "$label" in -*|*-) return 1 ;; esac + done +} + +# The value is written into an nginx listen directive, so it must be exactly +# one address or hostname; brackets are accepted around an IPv6 address only. +invalid_bind_host() { + echo "BIND_HOST must be an IP address or hostname: ${BIND_HOST}" >&2 + exit 1 +} case "$bind_host" in - ''|*[!A-Za-z0-9.:_-]*) - echo "BIND_HOST must be an IP address or hostname: ${BIND_HOST}" >&2 - exit 1 - ;; + \[*\]) bind_host="${bind_host#\[}"; bind_host="${bind_host%\]}"; is_ipv6 "$bind_host" || invalid_bind_host ;; + *[][]*) invalid_bind_host ;; + *:*) is_ipv6 "$bind_host" || invalid_bind_host ;; + *) is_ipv4 "$bind_host" || is_hostname "$bind_host" || invalid_bind_host ;; esac case "$bind_host" in diff --git a/scripts/serve.sh b/scripts/serve.sh index 1999f1b8a8..4f3546cd22 100755 --- a/scripts/serve.sh +++ b/scripts/serve.sh @@ -266,6 +266,9 @@ stop_all() { _kill_repo_processes "next dev" _kill_repo_processes "next start" _kill_repo_processes "next-server" + # -s quit finds the master through the config's pid directive. A BIND_HOST + # copy rendered by nginx-local-conf.sh keeps the same one, and the tracked + # config also works for --stop, which skips rendering. nginx -c "$REPO_ROOT/docker/nginx/nginx.local.conf" -p "$REPO_ROOT" -s quit 2>/dev/null || true sleep 1 _kill_repo_nginx