From a253faefdf9c6e0d98b4a031c65a398f59085d6d Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Sun, 27 Sep 2026 11:58:54 +0900 Subject: [PATCH] fix(docker): give project-document uploads their own nginx location so they are not capped at 1 MB (#5934) * fix(docker): give project-document uploads their own nginx location POST /api/projects/{id}/documents is a multipart upload that Gateway accepts up to uploads.max_file_size (50 MiB by default), but no nginx location matched it. It fell through to the /api/ catch-all, which has no client_max_body_size, so nginx's 1m default answered a 2 MB PDF with a bare 413 before Gateway saw the request. Reproduced on nginx:alpine with the shipped config; the same body to /api/threads/{id}/uploads passed. Add a `~ ^/api/projects/[^/]+/documents` location to the Docker, make dev and Helm configs with the thread-uploads settings (100M, streamed body) and the 600s read timeout the catch-all already granted these routes. The catch-all keeps nginx's defaults. The shared location-block helpers move to tests/support/nginx_conf.py so the new test and the existing body-size test use one implementation. * docs(changelog): reference #5934 in the project-documents nginx entries --- CHANGELOG.md | 12 ++ CHANGELOG_zh.md | 8 ++ backend/tests/support/nginx_conf.py | 66 +++++++++++ .../tests/test_nginx_langgraph_body_size.py | 62 +---------- .../test_nginx_project_documents_body_size.py | 103 ++++++++++++++++++ .../deer-flow/templates/configmap-nginx.yaml | 17 +++ docker/nginx/nginx.conf | 20 ++++ docker/nginx/nginx.local.conf | 24 ++++ 8 files changed, 255 insertions(+), 57 deletions(-) create mode 100644 backend/tests/support/nginx_conf.py create mode 100644 backend/tests/test_nginx_project_documents_body_size.py diff --git a/CHANGELOG.md b/CHANGELOG.md index af91831449..b0439d824a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -323,6 +323,17 @@ This release closes that milestone with **181 merged pull requests**. ### Fixed +- **docker:** Project-document uploads larger than 1 MB no longer fail with a + bare nginx `413` through the unified entry point. `POST + /api/projects/{id}/documents` is a multipart upload that Gateway accepts up + to `uploads.max_file_size` (50 MiB by default), but no nginx location + matched it, so it fell through to the `/api/` catch-all and nginx's default + `client_max_body_size 1m` rejected the request before Gateway saw it — a + 2 MB PDF was refused while the same file uploaded fine into a thread. All + three maintained configs (Docker, `make dev`, Helm) now give + `/api/projects/{id}/documents` its own location with the thread-uploads + settings (100M ceiling, streamed request body) and the read timeout the + catch-all already granted; the catch-all itself keeps nginx's defaults. ([#5934]) - **docker:** The production stack (`make up` / `scripts/deploy.sh`) now starts on hosts with IPv6 disabled. `docker/nginx/nginx.conf` also listens on `[::]:2026`; on a kernel booted with `ipv6.disable=1` that listen makes nginx @@ -6156,4 +6167,5 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#5893]: https://github.com/bytedance/deer-flow/pull/5893 [#5900]: https://github.com/bytedance/deer-flow/pull/5900 [#5928]: https://github.com/bytedance/deer-flow/pull/5928 +[#5934]: https://github.com/bytedance/deer-flow/pull/5934 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index fd5b1887ab..7b4eca26c0 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -328,6 +328,13 @@ ### 修复 +- **Docker：** 通过统一入口上传超过 1 MB 的项目文档不再被 nginx 直接以 `413` 拒绝。 + `POST /api/projects/{id}/documents` 是 multipart 上传，Gateway 接受至多 + `uploads.max_file_size`（默认 50 MiB）的文件，但没有任何 nginx location 匹配它，于是请求 + 落到 `/api/` 兜底块，被 nginx 默认的 `client_max_body_size 1m` 在到达 Gateway 之前拒绝—— + 一个 2 MB 的 PDF 会被拒，而同一个文件上传到会话里却没问题。三份维护中的配置（Docker、 + `make dev`、Helm）现在都为 `/api/projects/{id}/documents` 单独设置 location，沿用会话上传的 + 设置（100M 上限、请求体流式转发）以及兜底块原本给予的读超时；兜底块自身保持 nginx 默认值。([#5934]) - **Docker：** 生产栈（`make up` / `scripts/deploy.sh`）现在可以在禁用 IPv6 的主机上启动。 `docker/nginx/nginx.conf` 同时监听 `[::]:2026`；在以 `ipv6.disable=1` 启动的内核上，这条监听会让 nginx 在启动时退出，容器因此反复重启，`make up` 永远无法就绪。开发用 compose 文件自 #2027 起 @@ -5232,3 +5239,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#5893]: https://github.com/bytedance/deer-flow/pull/5893 [#5900]: https://github.com/bytedance/deer-flow/pull/5900 [#5928]: https://github.com/bytedance/deer-flow/pull/5928 +[#5934]: https://github.com/bytedance/deer-flow/pull/5934 diff --git a/backend/tests/support/nginx_conf.py b/backend/tests/support/nginx_conf.py new file mode 100644 index 0000000000..5fed8dbead --- /dev/null +++ b/backend/tests/support/nginx_conf.py @@ -0,0 +1,66 @@ +"""Helpers for asserting on the three maintained nginx configs. + +The Docker production config, the local-dev config used by ``make dev`` and the +Kubernetes/Helm ConfigMap template carry the same routing and must not drift; +tests parametrize over ``NGINX_CONFIGS`` and inspect one ``location`` block at +a time so a directive that merely appears elsewhere in the file cannot satisfy +an assertion by accident. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +NGINX_CONFIGS = ( + "docker/nginx/nginx.conf", + "docker/nginx/nginx.local.conf", + "deploy/helm/deer-flow/templates/configmap-nginx.yaml", +) + +_SIZE_MULTIPLIERS = {"": 1, "k": 1024, "m": 1024**2, "g": 1024**3} +_DURATION_MULTIPLIERS = {"": 1, "s": 1, "m": 60, "h": 3600} + + +def read_config(path: str) -> str: + return (REPO_ROOT / path).read_text(encoding="utf-8") + + +def extract_location_block(content: str, location_selector: str) -> str: + """Return the single ``location { ... }`` block. + + Brace-depth matching keeps assertions inside that location, so the + neighboring uploads location (which already has the large-body settings) + cannot make another route's assertions pass. + """ + marker = re.compile(r"location\s+" + re.escape(location_selector) + r"\s*\{") + match = marker.search(content) + assert match, f"could not find `location {location_selector}` block" + + start = match.end() - 1 # index of the opening brace + depth = 0 + for i, ch in enumerate(content[start:], start=start): + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return content[start : i + 1] + + raise AssertionError(f"unbalanced braces in `location {location_selector}` block") + + +def parse_body_size_bytes(block: str) -> int: + match = re.search(r"client_max_body_size\s+(\d+)\s*([mMkKgG]?)\s*;", block) + assert match, "client_max_body_size value not found or not parseable" + value, unit = match.groups() + return int(value) * _SIZE_MULTIPLIERS[unit.lower()] + + +def parse_read_timeout_seconds(block: str) -> int: + # Anchored to the start of a line so a commented-out directive does not count. + match = re.search(r"^\s*proxy_read_timeout\s+(\d+)([smh]?)\s*;", block, re.M) + assert match, "no active proxy_read_timeout directive" + value, unit = match.groups() + return int(value) * _DURATION_MULTIPLIERS[unit] diff --git a/backend/tests/test_nginx_langgraph_body_size.py b/backend/tests/test_nginx_langgraph_body_size.py index add08766d9..5dee980add 100644 --- a/backend/tests/test_nginx_langgraph_body_size.py +++ b/backend/tests/test_nginx_langgraph_body_size.py @@ -30,17 +30,12 @@ used by ``make dev``, and the Kubernetes/Helm ConfigMap template. from __future__ import annotations -import re -from pathlib import Path - import pytest - -REPO_ROOT = Path(__file__).resolve().parents[2] -NGINX_CONFIGS = ( - "docker/nginx/nginx.conf", - "docker/nginx/nginx.local.conf", - "deploy/helm/deer-flow/templates/configmap-nginx.yaml", -) +from support.nginx_conf import NGINX_CONFIGS +from support.nginx_conf import extract_location_block as _extract_location_block +from support.nginx_conf import parse_body_size_bytes as _parse_body_size_bytes +from support.nginx_conf import parse_read_timeout_seconds as _parse_read_timeout_seconds +from support.nginx_conf import read_config as _read # Text prompts never carry binary file attachments (those go through the # dedicated uploads route), so the ceiling here is intentionally well below @@ -50,56 +45,9 @@ NGINX_CONFIGS = ( _MIN_EXPECTED_BODY_SIZE_BYTES = 5 * 1024 * 1024 _MAX_EXPECTED_BODY_SIZE_BYTES = 100 * 1024 * 1024 -_SIZE_MULTIPLIERS = {"": 1, "k": 1024, "m": 1024**2, "g": 1024**3} - # The read timeout /api/langgraph/ already allows for a model-bound request. _MIN_BLOCKING_READ_TIMEOUT_SECONDS = 600 -_DURATION_MULTIPLIERS = {"": 1, "s": 1, "m": 60, "h": 3600} - - -def _read(path: str) -> str: - return (REPO_ROOT / path).read_text(encoding="utf-8") - - -def _extract_location_block(content: str, location_selector: str) -> str: - """Extract a single nginx ``location { ... }`` block - by brace-depth matching, so assertions target only that location and - can't be satisfied by a directive that merely appears elsewhere in the - file (e.g. the neighboring uploads location, which already has both - settings and must not make the langgraph-route assertions pass by - accident).""" - marker = re.compile(r"location\s+" + re.escape(location_selector) + r"\s*\{") - match = marker.search(content) - assert match, f"could not find `location {location_selector}` block" - - start = match.end() - 1 # index of the opening brace - depth = 0 - for i, ch in enumerate(content[start:], start=start): - if ch == "{": - depth += 1 - elif ch == "}": - depth -= 1 - if depth == 0: - return content[start : i + 1] - - raise AssertionError(f"unbalanced braces in `location {location_selector}` block") - - -def _parse_body_size_bytes(block: str) -> int: - match = re.search(r"client_max_body_size\s+(\d+)\s*([mMkKgG]?)\s*;", block) - assert match, "client_max_body_size value not found or not parseable" - value, unit = match.groups() - return int(value) * _SIZE_MULTIPLIERS[unit.lower()] - - -def _parse_read_timeout_seconds(block: str) -> int: - # Anchored to the start of a line so a commented-out directive does not count. - match = re.search(r"^\s*proxy_read_timeout\s+(\d+)([smh]?)\s*;", block, re.M) - assert match, "no active proxy_read_timeout directive" - value, unit = match.groups() - return int(value) * _DURATION_MULTIPLIERS[unit] - @pytest.mark.parametrize("path", NGINX_CONFIGS) def test_langgraph_route_disables_request_buffering(path): diff --git a/backend/tests/test_nginx_project_documents_body_size.py b/backend/tests/test_nginx_project_documents_body_size.py new file mode 100644 index 0000000000..f4b3a7f25c --- /dev/null +++ b/backend/tests/test_nginx_project_documents_body_size.py @@ -0,0 +1,103 @@ +"""Project-document uploads through nginx must not be capped at nginx's 1m default. + +``POST /api/projects/{project_id}/documents`` is a multipart upload that Gateway +accepts up to ``uploads.max_file_size`` (50 MiB by default, the same limit as +thread uploads). No nginx location matched it, so it fell through to the +``/api/`` catch-all, which carries no ``client_max_body_size``: nginx answered +a 2 MB PDF with a bare 413 before Gateway ever saw the request. Reproduced on +``nginx:alpine`` with the shipped config -- the same body sent to +``/api/threads/{id}/uploads`` passed nginx's size check. + +The route needs its own location, with the uploads route's settings, in all +three maintained configs. +""" + +from __future__ import annotations + +import re + +import pytest +from support.nginx_conf import NGINX_CONFIGS, extract_location_block, parse_body_size_bytes, parse_read_timeout_seconds, read_config + +from app.gateway.routers.project_documents import router as project_documents_router +from app.gateway.routers.uploads import DEFAULT_MAX_FILE_SIZE + +PROJECT_DOCUMENTS_LOCATION = r"~ ^/api/projects/[^/]+/documents" +UPLOADS_LOCATION = r"~ ^/api/threads/[^/]+/uploads" + +# The /api/ catch-all these routes used to land on grants blocking Gateway calls +# this much; moving them to their own location must not take it away. +_MIN_READ_TIMEOUT_SECONDS = 600 + + +def _location_regex(selector: str) -> re.Pattern[str]: + """The PCRE nginx evaluates for a ``~`` location, as a Python pattern (identical for these anchors and classes).""" + assert selector.startswith("~ ") + return re.compile(selector[2:]) + + +def _router_paths() -> list[str]: + """Concrete request paths for every route the project-documents router serves.""" + paths = [] + for route in project_documents_router.routes: + path = route.path.replace("{project_id}", "proj-42").replace("{document_id}", "doc-7").replace("{thread_id}", "thread-9") + paths.append(path) + assert paths, "project-documents router has no routes" + return paths + + +@pytest.mark.parametrize("path", NGINX_CONFIGS) +def test_project_documents_route_allows_the_upload_limit_gateway_enforces(path): + """The location must admit at least what Gateway accepts, and stream it.""" + block = extract_location_block(read_config(path), PROJECT_DOCUMENTS_LOCATION) + + size_bytes = parse_body_size_bytes(block) + uploads_size_bytes = parse_body_size_bytes(extract_location_block(read_config(path), UPLOADS_LOCATION)) + + assert size_bytes >= DEFAULT_MAX_FILE_SIZE, f"{path}: project documents client_max_body_size is {size_bytes} bytes but Gateway accepts {DEFAULT_MAX_FILE_SIZE} by default; nginx would 413 a file Gateway allows" + assert size_bytes == uploads_size_bytes, f"{path}: project documents and thread uploads share uploads.max_file_size, so their nginx ceilings must match" + assert "proxy_request_buffering off;" in block, f"{path}: uploads must stream to Gateway instead of spooling to client_body_temp" + + +@pytest.mark.parametrize("path", NGINX_CONFIGS) +def test_project_documents_route_keeps_the_catchall_read_timeout(path): + """attach-to-thread copies into a sandbox and content downloads stream; both used to get /api/'s 600s.""" + block = extract_location_block(read_config(path), PROJECT_DOCUMENTS_LOCATION) + catchall = extract_location_block(read_config(path), "/api/") + + assert parse_read_timeout_seconds(block) >= max(_MIN_READ_TIMEOUT_SECONDS, parse_read_timeout_seconds(catchall)) + + +@pytest.mark.parametrize("path", NGINX_CONFIGS) +def test_project_documents_location_matches_every_router_path_and_nothing_else(path): + """The regex must cover the whole router (the upload is one of five routes) without swallowing the project routes above it.""" + content = read_config(path) + extract_location_block(content, PROJECT_DOCUMENTS_LOCATION) # the selector under test is really in this file + pattern = _location_regex(PROJECT_DOCUMENTS_LOCATION) + + for request_path in _router_paths(): + assert pattern.search(request_path), f"{path}: {request_path} would fall through to /api/ and its 1m default" + for other in ("/api/projects", "/api/projects/proj-42", "/api/projects/proj-42/threads", "/api/projectsx/documents", "/api/threads/t/uploads"): + assert not pattern.search(other), f"{path}: {other} must not be routed as a project document" + + +@pytest.mark.parametrize("path", NGINX_CONFIGS) +def test_project_documents_location_proxies_like_its_neighbours(path): + """Same upstream and forwarded headers as the uploads route, so only the size policy differs from /api/.""" + content = read_config(path) + block = extract_location_block(content, PROJECT_DOCUMENTS_LOCATION) + uploads = extract_location_block(content, UPLOADS_LOCATION) + + def directives(text: str) -> set[str]: + return {line.strip() for line in text.splitlines() if re.match(r"\s*(proxy_pass|proxy_http_version|proxy_set_header)\b", line)} + + assert directives(block) == directives(uploads) + + +@pytest.mark.parametrize("path", NGINX_CONFIGS) +def test_api_catchall_still_keeps_nginx_default_body_policy(path): + """The fix is a dedicated location, not a blanket raise on /api/ (auth, config writes and JSON routes stay at the default).""" + block = extract_location_block(read_config(path), "/api/") + + assert "client_max_body_size" not in block + assert "proxy_request_buffering" not in block diff --git a/deploy/helm/deer-flow/templates/configmap-nginx.yaml b/deploy/helm/deer-flow/templates/configmap-nginx.yaml index 35ea3a0a02..5809813e91 100644 --- a/deploy/helm/deer-flow/templates/configmap-nginx.yaml +++ b/deploy/helm/deer-flow/templates/configmap-nginx.yaml @@ -160,6 +160,23 @@ data: proxy_request_buffering off; } + # Project documents — multipart upload plus the promote/attach/content/ + # delete routes. Without its own location the upload fell through to + # /api/ and nginx's 1m default returned 413 before Gateway saw it, + # although Gateway allows uploads.max_file_size (50 MiB by default). + # Same ceiling as uploads; the read timeout keeps what /api/ granted. + location ~ ^/api/projects/[^/]+/documents { + proxy_pass http://gateway_upstream; + proxy_http_version 1.1; + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $forwarded_proto; + client_max_body_size 100M; + proxy_request_buffering off; + proxy_read_timeout 600s; + } + location ~ ^/api/threads { proxy_pass http://gateway_upstream; proxy_http_version 1.1; diff --git a/docker/nginx/nginx.conf b/docker/nginx/nginx.conf index 516e05dfd5..6916fd7585 100644 --- a/docker/nginx/nginx.conf +++ b/docker/nginx/nginx.conf @@ -201,6 +201,26 @@ http { # Disable response buffering to avoid permission errors } + # Custom API: Project documents (multipart upload, promote-from-thread, + # attach-to-thread, content download, delete). Without its own location + # the upload fell through to /api/ and nginx's 1m default rejected + # anything larger with a 413 before Gateway saw it, although Gateway + # allows uploads.max_file_size (50 MiB by default) on this route. Same + # ceiling and streaming settings as the thread uploads route above; the + # read timeout keeps what /api/ already granted these routes. + location ~ ^/api/projects/[^/]+/documents { + proxy_pass http://$gateway_upstream; + proxy_http_version 1.1; + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $forwarded_proto; + + client_max_body_size 100M; + proxy_request_buffering off; + proxy_read_timeout 600s; + } + # Live browser stream is a WebSocket upgrade. It must be matched before # the generic /api/threads regex below (which omits Upgrade/Connection # forwarding and would downgrade it to plain HTTP). diff --git a/docker/nginx/nginx.local.conf b/docker/nginx/nginx.local.conf index 0f596682ec..b91ddb7c07 100644 --- a/docker/nginx/nginx.local.conf +++ b/docker/nginx/nginx.local.conf @@ -206,6 +206,30 @@ http { proxy_cache off; } + # Custom API: Project documents (multipart upload, promote-from-thread, + # attach-to-thread, content download, delete). Without its own location + # the upload fell through to /api/ and nginx's 1m default rejected + # anything larger with a 413 before Gateway saw it, although Gateway + # allows uploads.max_file_size (50 MiB by default) on this route. Same + # ceiling and streaming settings as the thread uploads route above; the + # read timeout keeps what /api/ already granted these routes. + location ~ ^/api/projects/[^/]+/documents { + proxy_pass http://gateway; + proxy_http_version 1.1; + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + + client_max_body_size 100M; + proxy_request_buffering off; + proxy_read_timeout 600s; + + # Disable response buffering to avoid permission errors + proxy_buffering off; + proxy_cache off; + } + # Live browser stream is a WebSocket upgrade. It must be matched before # the generic /api/threads regex below (which omits Upgrade/Connection # forwarding and would downgrade it to plain HTTP).