mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(config): guard numeric config fields that accepted silently-broken values (#6026)
* fix(config): guard numeric config fields that accepted silently-broken values
Ten numeric fields across six config models admitted zero, negatives, or
booleans (YAML/JSON true coerces to 1) and only failed at runtime — or kept
running with a nonsense value:
- CircuitBreakerConfig: failure_threshold >= 1, recovery_timeout_sec > 0
- McpServerConfig: tool_call_timeout > 0, session_init_timeout > 0
- McpOAuthConfig.refresh_skew_seconds >= 0 (0 stays valid)
- ModelConfig.stream_chunk_timeout > 0; context_window keeps gt=0 and
gains boolean rejection (review follow-up)
- RunEventsConfig.max_trace_content >= 1
- SandboxConfig.port 1..65535, idle_timeout >= 0 (0 = documented disable)
- SummarizationConfig.trim_tokens_to_summarize >= 1
Review follow-ups: boolean rejection extended to every bounded numeric
sibling in sandbox_config (replicas, acquire_timeout, burst_limit,
health_check_skip_seconds, bash_output_max_chars, read_file_output_max_chars,
ls_output_max_chars, bash_command_timeout, temporary_grant_ttl,
renewal_interval_seconds, ttl_multiplier) so the file is uniform, and to
model_config.context_window. Tests cover allow_inf_nan=False rejection
(float('inf')/'nan') and the refresh_skew_seconds=0 boundary.
Generated-by: ZCode (GLM, coding agent)
Authored-by: liwenjie200543 <liwenjie200543@users.noreply.github.com>
* style(config): apply Ruff formatting to numeric guards
Signed-off-by: Willem Jiang <willem.jiang@gmail.com>
---------
Signed-off-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: liwenjie200543 <liwenjie200543@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
fcff55a075
commit
f2f9e6f6fe
@ -7,7 +7,7 @@ from typing import Any, Literal, Self
|
||||
|
||||
import yaml
|
||||
from dotenv import load_dotenv
|
||||
from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, model_validator
|
||||
from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, field_validator, model_validator
|
||||
|
||||
from deerflow.config.acp_config import ACPAgentConfig, load_acp_config_from_dict
|
||||
from deerflow.config.agent_storage_config import AgentStorageConfig
|
||||
@ -77,8 +77,23 @@ CONFIG_FILE_DATABASE_DEFAULTS = {
|
||||
class CircuitBreakerConfig(BaseModel):
|
||||
"""Configuration for the LLM Circuit Breaker."""
|
||||
|
||||
failure_threshold: int = Field(default=5, description="Number of consecutive failures before tripping the circuit")
|
||||
recovery_timeout_sec: int = Field(default=60, description="Time in seconds before attempting to recover the circuit")
|
||||
failure_threshold: int = Field(
|
||||
default=5,
|
||||
ge=1,
|
||||
description="Number of consecutive failures before tripping the circuit",
|
||||
)
|
||||
recovery_timeout_sec: int = Field(
|
||||
default=60,
|
||||
gt=0,
|
||||
description="Time in seconds before attempting to recover the circuit",
|
||||
)
|
||||
|
||||
@field_validator("failure_threshold", "recovery_timeout_sec", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_circuit_settings(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be an integer, not a boolean")
|
||||
return value
|
||||
|
||||
|
||||
class LlmCallConfig(BaseModel):
|
||||
|
||||
@ -198,7 +198,19 @@ class McpOAuthConfig(BaseModel):
|
||||
token_type_field: str = Field(default="token_type", description="Field name containing token type in token response")
|
||||
expires_in_field: str = Field(default="expires_in", description="Field name containing expiry (seconds) in token response")
|
||||
default_token_type: str = Field(default="Bearer", description="Default token type when missing in token response")
|
||||
refresh_skew_seconds: int = Field(default=60, description="Refresh token this many seconds before expiry")
|
||||
refresh_skew_seconds: int = Field(
|
||||
default=60,
|
||||
ge=0,
|
||||
description="Refresh token this many seconds before expiry",
|
||||
)
|
||||
|
||||
@field_validator("refresh_skew_seconds", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_refresh_skew(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be an integer, not a boolean")
|
||||
return value
|
||||
|
||||
extra_token_params: dict[str, str] = Field(default_factory=dict, description="Additional form params sent to token endpoint")
|
||||
model_config = ConfigDict(extra="allow")
|
||||
|
||||
@ -232,10 +244,14 @@ class McpServerConfig(BaseModel):
|
||||
)
|
||||
tool_call_timeout: float | None = Field(
|
||||
default=None,
|
||||
gt=0,
|
||||
allow_inf_nan=False,
|
||||
description=("Timeout in seconds for individual stdio MCP tool calls and durable-task calls on every transport. Other HTTP/SSE tools use transport-level timeouts. None means no call-level timeout."),
|
||||
)
|
||||
session_init_timeout: float | None = Field(
|
||||
default=DEFAULT_MCP_SESSION_INIT_TIMEOUT,
|
||||
gt=0,
|
||||
allow_inf_nan=False,
|
||||
description=(
|
||||
"Timeout in seconds for MCP server bring-up: tool discovery (subprocess spawn + initialize + tools/list) "
|
||||
"and persistent stdio session initialization, plus ephemeral HTTP/SSE durable-task session "
|
||||
@ -243,6 +259,14 @@ class McpServerConfig(BaseModel):
|
||||
"construction or the task poller indefinitely. None means no timeout."
|
||||
),
|
||||
)
|
||||
|
||||
@field_validator("tool_call_timeout", "session_init_timeout", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_mcp_timeouts(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be a number, not a boolean")
|
||||
return value
|
||||
|
||||
task_toolsets: list[McpTaskToolsetConfig] = Field(
|
||||
default_factory=list,
|
||||
description="Ordinary submit/status/cancel tool groups managed by the durable MCP task runtime",
|
||||
|
||||
@ -184,6 +184,8 @@ class ModelConfig(BaseModel):
|
||||
)
|
||||
stream_chunk_timeout: float | None = Field(
|
||||
default=None,
|
||||
gt=0,
|
||||
allow_inf_nan=False,
|
||||
description=(
|
||||
"Maximum seconds to wait between successive streaming chunks before "
|
||||
"langchain-openai raises StreamChunkTimeoutError. None means use the "
|
||||
@ -192,6 +194,14 @@ class ModelConfig(BaseModel):
|
||||
"interactive endpoints. Has no effect on non-OpenAI-compatible providers."
|
||||
),
|
||||
)
|
||||
|
||||
@field_validator("stream_chunk_timeout", "context_window", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_numeric_settings(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be a number, not a boolean")
|
||||
return value
|
||||
|
||||
thinking: dict | None = Field(
|
||||
default_factory=lambda: None,
|
||||
description=(
|
||||
|
||||
@ -15,7 +15,7 @@ from __future__ import annotations
|
||||
|
||||
from typing import Literal
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
|
||||
|
||||
class RunEventsConfig(BaseModel):
|
||||
@ -25,8 +25,17 @@ class RunEventsConfig(BaseModel):
|
||||
)
|
||||
max_trace_content: int = Field(
|
||||
default=10240,
|
||||
ge=1,
|
||||
description="Maximum trace content size in bytes before truncation (db backend only).",
|
||||
)
|
||||
|
||||
@field_validator("max_trace_content", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_max_trace_content(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be an integer, not a boolean")
|
||||
return value
|
||||
|
||||
track_token_usage: bool = Field(
|
||||
default=True,
|
||||
description="Whether RunJournal should accumulate token counts to RunRow.",
|
||||
|
||||
@ -36,6 +36,14 @@ class SandboxNetworkConfig(BaseModel):
|
||||
le=3600,
|
||||
description="Lifetime in seconds for the temporary approval choice.",
|
||||
)
|
||||
|
||||
@field_validator("temporary_grant_ttl", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_temporary_grant_ttl(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be an integer, not a boolean")
|
||||
return value
|
||||
|
||||
proxy_image: str = Field(
|
||||
default="ghcr.io/bytedance/deer-flow-sandbox-network-proxy:latest",
|
||||
min_length=1,
|
||||
@ -112,6 +120,14 @@ class SandboxOwnershipConfig(BaseModel):
|
||||
allow_inf_nan=False,
|
||||
description="Lease TTL as a multiple of renewal_interval_seconds. At least 2, so a single missed renewal (slow host, brief Redis blip) cannot expire a live owner's lease. Default 4 tolerates three consecutive misses.",
|
||||
)
|
||||
|
||||
@field_validator("renewal_interval_seconds", "ttl_multiplier", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_ownership_settings(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be a number, not a boolean")
|
||||
return value
|
||||
|
||||
key_prefix: str = Field(
|
||||
default="deerflow:sandbox:owner",
|
||||
description="Redis key prefix for ownership leases. Only applies to the redis ownership type.",
|
||||
@ -204,6 +220,8 @@ class SandboxConfig(BaseModel):
|
||||
)
|
||||
port: int | None = Field(
|
||||
default=None,
|
||||
ge=1,
|
||||
le=65535,
|
||||
description="Base port for sandbox containers",
|
||||
)
|
||||
replicas: int | None = Field(
|
||||
@ -231,8 +249,29 @@ class SandboxConfig(BaseModel):
|
||||
)
|
||||
idle_timeout: int | None = Field(
|
||||
default=None,
|
||||
ge=0,
|
||||
description="Idle timeout in seconds before released warm sandboxes/VMs are stopped (default: 600 = 10 minutes). Set to 0 to disable.",
|
||||
)
|
||||
|
||||
@field_validator(
|
||||
"port",
|
||||
"replicas",
|
||||
"acquire_timeout",
|
||||
"burst_limit",
|
||||
"idle_timeout",
|
||||
"health_check_skip_seconds",
|
||||
"bash_output_max_chars",
|
||||
"read_file_output_max_chars",
|
||||
"ls_output_max_chars",
|
||||
"bash_command_timeout",
|
||||
mode="before",
|
||||
)
|
||||
@classmethod
|
||||
def _reject_boolean_numeric_settings(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be a number, not a boolean")
|
||||
return value
|
||||
|
||||
health_check_skip_seconds: float | None = Field(
|
||||
default=None,
|
||||
ge=0,
|
||||
|
||||
@ -3,7 +3,7 @@
|
||||
import math
|
||||
from typing import Literal
|
||||
|
||||
from pydantic import BaseModel, Field, model_validator
|
||||
from pydantic import BaseModel, Field, field_validator, model_validator
|
||||
|
||||
ContextSizeType = Literal["fraction", "tokens", "messages"]
|
||||
DEFAULT_SKILL_FILE_READ_TOOL_NAMES: tuple[str, ...] = ("read_file", "read", "view", "cat")
|
||||
@ -82,8 +82,17 @@ class SummarizationConfig(BaseModel):
|
||||
)
|
||||
trim_tokens_to_summarize: int | None = Field(
|
||||
default=4000,
|
||||
ge=1,
|
||||
description="Maximum tokens to keep when preparing messages for summarization. Pass null to skip trimming.",
|
||||
)
|
||||
|
||||
@field_validator("trim_tokens_to_summarize", mode="before")
|
||||
@classmethod
|
||||
def _reject_boolean_trim_tokens(cls, value: object) -> object:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("must be an integer, not a boolean")
|
||||
return value
|
||||
|
||||
summary_prompt: str | None = Field(
|
||||
default=None,
|
||||
description="Custom prompt template for generating summaries. If not provided, uses the default LangChain prompt.",
|
||||
|
||||
166
backend/tests/test_numeric_config_guards.py
Normal file
166
backend/tests/test_numeric_config_guards.py
Normal file
@ -0,0 +1,166 @@
|
||||
# SPDX-License-Identifier: MIT
|
||||
"""Numeric config fields that previously accepted silently-broken values.
|
||||
|
||||
Each guarded field used to admit zero, negatives, or booleans (YAML/JSON
|
||||
``true`` coerces to ``1``) and only failed later at runtime — or worse, kept
|
||||
running with a nonsense value. Bounds and the boolean rejection mirror the
|
||||
guards already shipped for ``stream_bridge_config`` (heartbeat interval) and
|
||||
``database_config`` (pool settings).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.app_config import CircuitBreakerConfig
|
||||
from deerflow.config.extensions_config import McpOAuthConfig, McpServerConfig
|
||||
from deerflow.config.model_config import ModelConfig
|
||||
from deerflow.config.run_events_config import RunEventsConfig
|
||||
from deerflow.config.sandbox_config import SandboxConfig
|
||||
from deerflow.config.summarization_config import SummarizationConfig
|
||||
|
||||
|
||||
class TestCircuitBreakerGuards:
|
||||
def test_defaults_pass(self) -> None:
|
||||
config = CircuitBreakerConfig()
|
||||
assert config.failure_threshold == 5
|
||||
assert config.recovery_timeout_sec == 60
|
||||
|
||||
@pytest.mark.parametrize("field", ["failure_threshold", "recovery_timeout_sec"])
|
||||
@pytest.mark.parametrize("bad", [0, -1, True])
|
||||
def test_rejects_non_positive_and_boolean(self, field: str, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
CircuitBreakerConfig(**{field: bad})
|
||||
|
||||
|
||||
class TestMcpTimeoutGuards:
|
||||
def test_defaults_pass(self) -> None:
|
||||
config = McpServerConfig()
|
||||
assert config.tool_call_timeout is None
|
||||
assert config.session_init_timeout is not None
|
||||
|
||||
@pytest.mark.parametrize("field", ["tool_call_timeout", "session_init_timeout"])
|
||||
@pytest.mark.parametrize("bad", [0, -1.0, True])
|
||||
def test_rejects_non_positive_and_boolean(self, field: str, bad: float | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
McpServerConfig(**{field: bad})
|
||||
|
||||
def test_zero_point_five_and_none_are_valid(self) -> None:
|
||||
config = McpServerConfig(tool_call_timeout=0.5, session_init_timeout=None)
|
||||
assert config.tool_call_timeout == 0.5
|
||||
assert config.session_init_timeout is None
|
||||
|
||||
@pytest.mark.parametrize("bad", [-1, True])
|
||||
def test_refresh_skew_rejects_negative_and_boolean(self, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
McpOAuthConfig(refresh_skew_seconds=bad)
|
||||
|
||||
|
||||
class TestStreamChunkTimeoutGuard:
|
||||
def test_none_default_passes(self) -> None:
|
||||
config = ModelConfig(name="m", use="default", model="gpt-x", stream_chunk_timeout=None)
|
||||
assert config.stream_chunk_timeout is None
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, -1.0, True])
|
||||
def test_rejects_non_positive_and_boolean(self, bad: float | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", stream_chunk_timeout=bad)
|
||||
|
||||
|
||||
class TestMaxTraceContentGuard:
|
||||
def test_default_passes(self) -> None:
|
||||
assert RunEventsConfig().max_trace_content == 10240
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, -1, True])
|
||||
def test_rejects_non_positive_and_boolean(self, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
RunEventsConfig(**{"max_trace_content": bad})
|
||||
|
||||
|
||||
class TestSandboxPortAndIdleTimeoutGuards:
|
||||
def test_unset_defaults_pass(self) -> None:
|
||||
config = SandboxConfig(use="local")
|
||||
assert config.port is None
|
||||
assert config.idle_timeout is None
|
||||
|
||||
def test_zero_idle_timeout_is_documented_valid(self) -> None:
|
||||
assert SandboxConfig(use="local", idle_timeout=0).idle_timeout == 0
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, 65536, -1, True])
|
||||
def test_port_rejects_out_of_range_and_boolean(self, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", port=bad)
|
||||
|
||||
@pytest.mark.parametrize("bad", [-1, True])
|
||||
def test_idle_timeout_rejects_negative_and_boolean(self, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", idle_timeout=bad)
|
||||
|
||||
|
||||
class TestSiblingBoolRejection:
|
||||
"""Same-class siblings with bounds but no boolean rejection, flagged in review."""
|
||||
|
||||
def test_sandbox_numeric_fields_reject_booleans(self) -> None:
|
||||
import pytest
|
||||
|
||||
from deerflow.config.sandbox_config import SandboxOwnershipConfig
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", replicas=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", acquire_timeout=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", burst_limit=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", health_check_skip_seconds=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", bash_output_max_chars=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", read_file_output_max_chars=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", ls_output_max_chars=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxConfig(use="local", bash_command_timeout=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxOwnershipConfig(renewal_interval_seconds=True)
|
||||
with pytest.raises(ValidationError):
|
||||
SandboxOwnershipConfig(ttl_multiplier=True)
|
||||
|
||||
def test_ownership_defaults_still_pass(self) -> None:
|
||||
from deerflow.config.sandbox_config import SandboxOwnershipConfig
|
||||
|
||||
config = SandboxOwnershipConfig()
|
||||
assert config.renewal_interval_seconds == 30.0
|
||||
assert config.ttl_multiplier == 4.0
|
||||
|
||||
def test_context_window_rejects_booleans_and_nonpositive(self) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", context_window=True)
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", context_window=0)
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", context_window=-1)
|
||||
|
||||
def test_allow_inf_nan_rejection(self) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
McpServerConfig(tool_call_timeout=float("inf"))
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", stream_chunk_timeout=float("inf"))
|
||||
with pytest.raises(ValidationError):
|
||||
ModelConfig(name="m", use="default", model="gpt-x", stream_chunk_timeout=float("nan"))
|
||||
|
||||
def test_refresh_skew_zero_is_a_valid_boundary(self) -> None:
|
||||
config = McpOAuthConfig(token_url="https://sso.example.org/token", refresh_skew_seconds=0)
|
||||
assert config.refresh_skew_seconds == 0
|
||||
|
||||
|
||||
class TestTrimTokensToSummarizeGuard:
|
||||
def test_default_and_none_pass(self) -> None:
|
||||
assert SummarizationConfig().trim_tokens_to_summarize == 4000
|
||||
assert SummarizationConfig(trim_tokens_to_summarize=None).trim_tokens_to_summarize is None
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, -1, True])
|
||||
def test_rejects_non_positive_and_boolean(self, bad: int | bool) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
SummarizationConfig(**{"trim_tokens_to_summarize": bad})
|
||||
Loading…
x
Reference in New Issue
Block a user