mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-09 16:08:31 +00:00
fix(gateway): clamp client-supplied recursion_limit to prevent runaway runs (#3903)
Some checks are pending
Backend Blocking IO / backend-blocking-io (push) Waiting to run
Unit Tests / backend-unit-tests (push) Waiting to run
E2E Tests / e2e-tests (push) Waiting to run
Frontend Unit Tests / frontend-unit-tests (push) Waiting to run
Lint Check / lint-backend (push) Waiting to run
Lint Check / lint-frontend (push) Waiting to run
Replay E2E (front-back contract) / Layer 1 — backend golden (no API key) (push) Waiting to run
Replay E2E (front-back contract) / Layer 2 — full-stack render (no API key) (push) Waiting to run
Some checks are pending
Backend Blocking IO / backend-blocking-io (push) Waiting to run
Unit Tests / backend-unit-tests (push) Waiting to run
E2E Tests / e2e-tests (push) Waiting to run
Frontend Unit Tests / frontend-unit-tests (push) Waiting to run
Lint Check / lint-backend (push) Waiting to run
Lint Check / lint-frontend (push) Waiting to run
Replay E2E (front-back contract) / Layer 1 — backend golden (no API key) (push) Waiting to run
Replay E2E (front-back contract) / Layer 2 — full-stack render (no API key) (push) Waiting to run
build_run_config() copied every top-level request key (except
configurable/context) verbatim into the LangGraph RunnableConfig, including
recursion_limit. The server default of 100 was fully overridable by the caller
with no upper bound, so a request like {"config": {"recursion_limit": 100000000}}
could make a single run execute effectively unbounded LangGraph super-steps
(each >= 1 LLM call), enabling runaway API cost / DoS.
Validate the client value server-side and clamp it into a safe range:
- valid positive ints are capped at a configurable ceiling
(AppConfig.max_recursion_limit, default 1000 to match the existing
frontend/public-skill default so legitimate deep runs are unaffected)
- invalid/non-positive/bool/None values fall back to the 100 server default
- applied on both the configurable and the LangGraph >= 0.6.0 context paths
- WARNING logged on clamp for observability
Add unit tests (including a configurable-ceiling case), expose
max_recursion_limit in config.example.yaml (config_version 16), and document
the ceiling/fallback in backend/docs/API.md.
Co-authored-by: DengY11 <DengY11@users.noreply.github.com>
This commit is contained in:
parent
21b3510226
commit
4e6248f013
5 changed files with 136 additions and 2 deletions
|
|
@ -204,6 +204,41 @@ def resolve_agent_factory(assistant_id: str | None):
|
|||
return make_lead_agent
|
||||
|
||||
|
||||
# Lead-agent recursion budget bounds. The Gateway must NOT trust a
|
||||
# client-supplied ``recursion_limit`` verbatim: an arbitrarily large value lets
|
||||
# a single run execute unbounded LangGraph super-steps (each at least one LLM
|
||||
# call), enabling runaway API cost / DoS. ``_DEFAULT_RECURSION_LIMIT`` is the
|
||||
# server default when the client sends nothing; the hard ceiling any client
|
||||
# value is clamped to is configurable via ``AppConfig.max_recursion_limit``.
|
||||
_DEFAULT_RECURSION_LIMIT = 100
|
||||
_DEFAULT_MAX_RECURSION_LIMIT = 1000
|
||||
|
||||
|
||||
def _resolve_max_recursion_limit() -> int:
|
||||
"""Resolve the clamp ceiling from ``AppConfig.max_recursion_limit``.
|
||||
|
||||
Falls back to ``_DEFAULT_MAX_RECURSION_LIMIT`` when the app config cannot be
|
||||
loaded (e.g. no ``config.yaml`` in a bare unit-test environment) so that the
|
||||
clamp still applies rather than crashing the run-config assembly.
|
||||
"""
|
||||
try:
|
||||
return get_app_config().max_recursion_limit
|
||||
except Exception:
|
||||
return _DEFAULT_MAX_RECURSION_LIMIT
|
||||
|
||||
|
||||
def _clamp_recursion_limit(value: Any, max_limit: int) -> int:
|
||||
"""Clamp a client-supplied ``recursion_limit`` into a safe server range.
|
||||
|
||||
Non-integer values (including ``bool``, an ``int`` subclass) and non-positive
|
||||
values fall back to ``_DEFAULT_RECURSION_LIMIT``; valid positive integers are
|
||||
capped at ``max_limit`` (from ``AppConfig.max_recursion_limit``).
|
||||
"""
|
||||
if isinstance(value, bool) or not isinstance(value, int) or value <= 0:
|
||||
return _DEFAULT_RECURSION_LIMIT
|
||||
return min(value, max_limit)
|
||||
|
||||
|
||||
def build_run_config(
|
||||
thread_id: str,
|
||||
request_config: dict[str, Any] | None,
|
||||
|
|
@ -233,7 +268,7 @@ def build_run_config(
|
|||
# subagent inside ONE lead tools-node step, and subagents enforce their own
|
||||
# limit via `subagents.max_turns`. Do not conflate this 100 with the
|
||||
# general-purpose subagent's max_turns.
|
||||
config: dict[str, Any] = {"recursion_limit": 100}
|
||||
config: dict[str, Any] = {"recursion_limit": _DEFAULT_RECURSION_LIMIT}
|
||||
if request_config:
|
||||
# LangGraph >= 0.6.0 introduced ``context`` as the preferred way to
|
||||
# pass thread-level data and rejects requests that include both
|
||||
|
|
@ -262,6 +297,22 @@ def build_run_config(
|
|||
for k, v in request_config.items():
|
||||
if k not in ("configurable", "context"):
|
||||
config[k] = v
|
||||
# Never trust a client-supplied recursion_limit verbatim: clamp it to a
|
||||
# safe server range so a single run cannot execute unbounded LangGraph
|
||||
# super-steps (runaway LLM cost / DoS). Applied after the passthrough so
|
||||
# it overrides whatever the client sent.
|
||||
if "recursion_limit" in request_config:
|
||||
max_limit = _resolve_max_recursion_limit()
|
||||
clamped = _clamp_recursion_limit(request_config["recursion_limit"], max_limit)
|
||||
if clamped != request_config["recursion_limit"]:
|
||||
logger.warning(
|
||||
"build_run_config: clamped client recursion_limit %r -> %d (max %d). thread_id=%s",
|
||||
request_config["recursion_limit"],
|
||||
clamped,
|
||||
max_limit,
|
||||
thread_id,
|
||||
)
|
||||
config["recursion_limit"] = clamped
|
||||
else:
|
||||
config["configurable"] = {"thread_id": thread_id}
|
||||
|
||||
|
|
|
|||
|
|
@ -108,7 +108,10 @@ in a single run. The unified Gateway path defaults to `100` in
|
|||
`build_run_config` (see `backend/app/gateway/services.py`), which is a safer
|
||||
starting point for plan-mode or subagent-heavy runs. Clients can still set
|
||||
`recursion_limit` explicitly in the request body; increase it if you run deeply
|
||||
nested subagent graphs.
|
||||
nested subagent graphs. For safety, the Gateway clamps any client-supplied value
|
||||
to a configurable server ceiling (`max_recursion_limit` in `config.yaml`,
|
||||
default `1000`) so a single run cannot execute unbounded graph steps (runaway
|
||||
LLM cost / DoS); invalid or non-positive values fall back to the `100` default.
|
||||
|
||||
**Configurable Options:**
|
||||
- `model_name` (string): Override the default model
|
||||
|
|
|
|||
|
|
@ -100,6 +100,11 @@ class AppConfig(BaseModel):
|
|||
)
|
||||
token_usage: TokenUsageConfig = Field(default_factory=TokenUsageConfig, description="Token usage tracking configuration")
|
||||
token_budget: TokenBudgetConfig = Field(default_factory=TokenBudgetConfig, description="Token Budget tracking and limits configuration.")
|
||||
max_recursion_limit: int = Field(
|
||||
default=1000,
|
||||
ge=1,
|
||||
description="Hard server-side ceiling for a client-supplied run recursion_limit. Client values above this are clamped; prevents runaway LangGraph super-steps (LLM cost / DoS).",
|
||||
)
|
||||
models: list[ModelConfig] = Field(default_factory=list, description="Available models")
|
||||
sandbox: SandboxConfig = Field(
|
||||
description=format_field_description(
|
||||
|
|
|
|||
|
|
@ -208,6 +208,70 @@ def test_build_run_config_with_overrides():
|
|||
assert config["metadata"]["user"] == "alice"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# recursion_limit clamping: the Gateway must not trust a client-supplied
|
||||
# recursion_limit verbatim (runaway LLM cost / DoS). See build_run_config.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_build_run_config_clamps_excessive_recursion_limit(_stub_app_config):
|
||||
"""A huge client recursion_limit is capped at the configured ceiling (default 1000)."""
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config("thread-1", {"recursion_limit": 100_000_000}, None)
|
||||
assert config["recursion_limit"] == 1000
|
||||
|
||||
|
||||
def test_build_run_config_ceiling_is_configurable(_stub_app_config):
|
||||
"""The clamp ceiling comes from AppConfig.max_recursion_limit, not a hardcoded value."""
|
||||
from app.gateway.services import build_run_config
|
||||
from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config
|
||||
|
||||
set_app_config(AppConfig.model_validate({"sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}, "max_recursion_limit": 300}))
|
||||
try:
|
||||
config = build_run_config("thread-1", {"recursion_limit": 100_000_000}, None)
|
||||
assert config["recursion_limit"] == 300
|
||||
finally:
|
||||
reset_app_config()
|
||||
|
||||
|
||||
def test_build_run_config_allows_recursion_limit_at_ceiling(_stub_app_config):
|
||||
"""A value at the configured ceiling is preserved unchanged."""
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config("thread-1", {"recursion_limit": 1000}, None)
|
||||
assert config["recursion_limit"] == 1000
|
||||
|
||||
|
||||
def test_build_run_config_preserves_reasonable_recursion_limit(_stub_app_config):
|
||||
"""A modest client value below the ceiling is honoured as-is."""
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config("thread-1", {"recursion_limit": 250}, None)
|
||||
assert config["recursion_limit"] == 250
|
||||
|
||||
|
||||
def test_build_run_config_rejects_invalid_recursion_limit(_stub_app_config):
|
||||
"""Non-positive / non-int / bool values fall back to the server default."""
|
||||
from app.gateway.services import _DEFAULT_RECURSION_LIMIT, build_run_config
|
||||
|
||||
for bad in (0, -5, "1000", 3.5, True, None):
|
||||
config = build_run_config("thread-1", {"recursion_limit": bad}, None)
|
||||
assert config["recursion_limit"] == _DEFAULT_RECURSION_LIMIT, bad
|
||||
|
||||
|
||||
def test_build_run_config_clamps_recursion_limit_with_context(_stub_app_config):
|
||||
"""Clamping also applies on the LangGraph >= 0.6.0 context passthrough path."""
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config(
|
||||
"thread-1",
|
||||
{"context": {"thread_id": "thread-1"}, "recursion_limit": 999_999},
|
||||
None,
|
||||
)
|
||||
assert config["recursion_limit"] == 1000
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Regression tests for issue #1644:
|
||||
# assistant_id not mapped to agent_name → custom agent SOUL.md never loaded
|
||||
|
|
|
|||
|
|
@ -47,6 +47,17 @@ token_budget:
|
|||
warn_threshold: 0.8 # Warn at 80% of the budget
|
||||
hard_stop_threshold: 1.0 # Force stop at 100% of the budget
|
||||
|
||||
# ============================================================================
|
||||
# Recursion Limit — Hard ceiling for a client-supplied run recursion_limit
|
||||
# ============================================================================
|
||||
# A run's recursion_limit caps the number of LangGraph super-steps (each is at
|
||||
# least one LLM call). The Gateway never trusts a client-supplied value
|
||||
# verbatim: any value above this ceiling is clamped down to it, preventing
|
||||
# runaway API cost / DoS. Invalid or non-positive client values fall back to
|
||||
# the server default of 100. Raise this only if you legitimately run very
|
||||
# deeply nested subagent graphs.
|
||||
max_recursion_limit: 1000
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Models Configuration
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue