mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(mcp): fall back when a token response carries an unusable expires_in (#6473)
* fix(mcp): fall back when a token response carries an unusable expires_in
OAuthTokenManager._fetch_token() converted the token endpoint's
expires_in to a timedelta while guarding only TypeError/ValueError. Two
server-controlled inputs raised OverflowError, which escaped the parser and
failed the whole token fetch, so the MCP server never received its
Authorization header:
* int(float('inf')) raises OverflowError. json.loads accepts Infinity, so a
non-finite expires_in reached it uncaught (NaN happened to be caught).
* int(1e30) succeeds, but timedelta is capped at 999_999_999 days and rejects
the magnitude with OverflowError. A large JSON integer literal behaves the
same way.
Catch OverflowError alongside TypeError/ValueError and treat a lifetime beyond
timedelta's own range as unusable, so both fall back to the documented
one-hour default like every other malformed value.
Fixes #6472
* fix(mcp): handle both token expiry overflow stages
---------
Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
7d54feea07
commit
be34cc46d0
@ -738,6 +738,7 @@ Tool-produced paths and URLs can be retained as short artifact handles across co
|
||||
|
||||
DeerFlow supports configurable MCP servers and skills to extend its capabilities.
|
||||
For HTTP/SSE MCP servers, OAuth token flows are supported (`client_credentials`, `refresh_token`).
|
||||
Missing, malformed, or out-of-range token response `expires_in` values use a one-hour default lifetime. This includes lifetimes that cannot be added to the current time without overflowing the expiry timestamp.
|
||||
Durable HTTP/SSE task status and cancellation calls select configured `user_auth` credentials using the persisted task owner, including after restart; per-request secrets are not retained for background calls. If a request-scoped credential overrides submit authentication, both credentials must authorize access to the same remote task.
|
||||
For stdio MCP servers, per-tool call timeouts can be configured with `tool_call_timeout`; durable background-task calls honor the same setting for HTTP/SSE servers as well.
|
||||
For stdio file outputs, a bare filename is linked to a uniquely matching file created or changed by that call. Filenames embedded in unrelated paths, including Windows backslash paths, are left intact.
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
### MCP System (`packages/harness/deerflow/mcp/`)
|
||||
|
||||
- Uses `langchain-mcp-adapters` `MultiServerMCPClient` for multi-server management
|
||||
- **OAuth expiry**: Missing, non-convertible or overflowing `expires_in` values use the one-hour default. Guard both `timedelta` construction and addition to one captured UTC `now`; avoid float-derived range ceilings. `tests/test_mcp_oauth.py` pins both overflow stages and the exact `datetime` boundary with a fixed clock.
|
||||
- **Long-running task foundation**: `mcp/tasks/` defines the protocol-neutral `McpTaskDriver` contract and normalized `TaskSnapshot` states (`submitted`, `working`, `input_required`, `completed`, `failed`, `cancelled`). A driver-supplied `poll_after_seconds` must be a finite positive number, validated at the `TaskSnapshot` boundary so every driver is held to the same invariant rather than each one guarding the consumer that turns the interval into a `timedelta`. `persistence/mcp_tasks/` owns the durable remote-handle mapping, poll schedule, notification state, lease owner, and separate consecutive poll/delivery error counters; `app/mcp_tasks/McpTaskService` performs status, cancellation, and notification work outside the Agent/LLM loop. Notification retries keep their idempotency attempt separate from the delivery-failure count, use capped exponential backoff, and stop after five failures; strict existing-thread admission dead-letters a deleted/mismatched target immediately. A status result is applied only when the worker still owns an unexpired lease, so a stale result cannot be written after expiry even before another worker reclaims the row. Every claim carries a fresh per-claim token, and every poll/cancel/notification release or snapshot apply is fenced by the owner plus that token, so a release that completes after the same service reclaims the row cannot clear the newer lease. Poll, cancel, and notification claims also have separate phase-level single-flight owners that protect only an ambiguous claim outcome: after an uncancelled timeout or caller cancellation, later scans do not start another claim while the outcome is unknown. Once the claim resolves, the phase owner is released immediately, and any returned rows are released through bounded, service-owned background work; per-claim token fencing prevents a late release from mutating a newer claim generation, so a stuck release does not stall the whole phase. While the claim outcome remains unresolved, later scans skip the phase and emit a warning rather than overlapping an ambiguous database claim. Once the claim resolves, the handoff may continue releasing returned rows in the background without blocking later scans. Lease expiry remains the cross-process crash fallback rather than the normal recovery for a live service's late claim. Poll timestamps and retry schedules are based on the remote call's completion time rather than the scan start. If submission succeeds but persistence fails or the caller is cancelled while persistence is in flight, the service best-effort cancels the remote task and preserves the original error or cancellation if that compensation also fails. The exact `uq_mcp_tasks_user_server_remote` conflict is different: an existing durable row already owns the remote handle, so the conflict surfaces without cancelling that tracked task. Unexpected per-task poll failures are isolated from sibling claims and remain recoverable through lease expiry; Gateway shutdown cancels the poller so a hung external status call cannot block process exit. Routine cancellation releases clear only the lease/token and preserve any pre-existing `last_cancel_error` or `notification_error`; cancellation itself is not persisted as a task failure diagnostic. Cancelling an in-flight poll releases only the owner- and per-claim-token-fenced lease and preserves its preclaim schedule and poll-failure state; real poll failures retain exponential backoff and tracking-degradation behavior. `input_required` and terminal states stop polling and become `notification_status=pending` for later Agent/UI delivery. Durable recovery requires a SQL database backend (`sqlite` or `postgres`); the in-memory backend leaves the repository/service unavailable. The runtime is startup-configured by `mcp_tasks` and disabled by default until a concrete driver is registered; this foundation does not alter ordinary MCP tool behavior on its own.
|
||||
- **Runtime availability boundary**: the installed process-local submitter is the source of truth for durable task-management tool exposure. `mcp_tasks` is startup-only; changing it on disk does not alter the live toolset until the Gateway restarts.
|
||||
- **Task OAuth lifetime**: `tasks/runtime.py` owns shared OAuth state only for enabled deployment servers with `task_toolsets`, from `set_mcp_task_config_snapshot(config)` until its `None` teardown. Use `_task_enabled_servers` for both snapshot and OAuth selection. Validate the frozen snapshot before reusing that state for discovery, ordinary tools, or durable calls. Cache resets must not discard it. Keep tokens and the cross-loop refresh lock together; rotate a private config copy, never the parsed snapshot or config file. Non-task servers remain reloadable and personal callers use independent managers, even on a name collision. This is not cross-process or cross-restart credential storage.
|
||||
|
||||
@ -14,6 +14,10 @@ from deerflow.mcp.headers import apply_header_overrides, header_spellings, illeg
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Used when the token endpoint omits ``expires_in`` or returns a value the
|
||||
# manager cannot turn into a lifetime.
|
||||
_DEFAULT_TOKEN_LIFETIME_SECONDS = 3600
|
||||
|
||||
|
||||
@dataclass
|
||||
class _OAuthToken:
|
||||
@ -224,13 +228,22 @@ class OAuthTokenManager:
|
||||
|
||||
token_type = str(payload.get(oauth.token_type_field, oauth.default_token_type) or oauth.default_token_type)
|
||||
|
||||
expires_in_raw = payload.get(oauth.expires_in_field, 3600)
|
||||
expires_in_raw = payload.get(oauth.expires_in_field, _DEFAULT_TOKEN_LIFETIME_SECONDS)
|
||||
try:
|
||||
expires_in = int(expires_in_raw)
|
||||
except (TypeError, ValueError):
|
||||
expires_in = 3600
|
||||
|
||||
expires_at = datetime.now(UTC) + timedelta(seconds=max(expires_in, 1))
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# ``OverflowError``: ``json.loads`` accepts ``Infinity``/``NaN``, and
|
||||
# ``int()`` rejects a non-finite float. Treat it like any other
|
||||
# unusable value instead of failing the whole token fetch.
|
||||
expires_in = _DEFAULT_TOKEN_LIFETIME_SECONDS
|
||||
now = datetime.now(UTC)
|
||||
try:
|
||||
expires_at = now + timedelta(seconds=max(expires_in, 1))
|
||||
except OverflowError:
|
||||
# Either timedelta construction or datetime addition can overflow.
|
||||
# Let the arithmetic enforce its exact range, avoiding a rounded
|
||||
# total_seconds() ceiling, and use the same instant for fallback.
|
||||
expires_at = now + timedelta(seconds=_DEFAULT_TOKEN_LIFETIME_SECONDS)
|
||||
return _OAuthToken(access_token=access_token, token_type=token_type, expires_at=expires_at)
|
||||
|
||||
|
||||
|
||||
@ -5,6 +5,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import logging
|
||||
import threading
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
@ -87,6 +88,99 @@ def test_oauth_token_manager_fetches_and_caches_token(monkeypatch):
|
||||
assert post_calls[0]["data"]["grant_type"] == "client_credentials"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def oauth_now(monkeypatch):
|
||||
now = datetime(2026, 1, 1, 12, 0, 0, 123456, tzinfo=UTC)
|
||||
|
||||
class _FixedDatetime(datetime):
|
||||
@classmethod
|
||||
def now(cls, tz):
|
||||
return now
|
||||
|
||||
monkeypatch.setattr("deerflow.mcp.oauth.datetime", _FixedDatetime)
|
||||
return now
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"expires_in",
|
||||
[
|
||||
float("inf"),
|
||||
float("-inf"),
|
||||
float("nan"),
|
||||
1e30,
|
||||
10**30,
|
||||
pytest.param(86_400_000_000_000, id="rounded-timedelta-limit"),
|
||||
pytest.param(10**12, id="datetime-overflow"),
|
||||
"not-a-number",
|
||||
],
|
||||
)
|
||||
def test_unusable_expires_in_falls_back_to_the_default_lifetime(monkeypatch, oauth_now, expires_in):
|
||||
"""A malformed ``expires_in`` must not abort the token fetch.
|
||||
|
||||
Cover conversion, ``timedelta`` construction and ``datetime`` addition:
|
||||
each can reject a lifetime that the token endpoint returns.
|
||||
"""
|
||||
post_calls: list[dict[str, Any]] = []
|
||||
|
||||
def _client_factory(*args, **kwargs):
|
||||
return _MockAsyncClient(
|
||||
payload={
|
||||
"access_token": "token-123",
|
||||
"token_type": "Bearer",
|
||||
"expires_in": expires_in,
|
||||
},
|
||||
post_calls=post_calls,
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
monkeypatch.setattr("httpx.AsyncClient", _client_factory)
|
||||
|
||||
config = ExtensionsConfig.model_validate(
|
||||
{
|
||||
"mcpServers": {
|
||||
"secure-http": {
|
||||
"enabled": True,
|
||||
"type": "http",
|
||||
"url": "https://api.example.com/mcp",
|
||||
"oauth": {
|
||||
"enabled": True,
|
||||
"token_url": "https://auth.example.com/oauth/token",
|
||||
"grant_type": "client_credentials",
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
)
|
||||
manager = OAuthTokenManager.from_extensions_config(config)
|
||||
|
||||
header = asyncio.run(manager.get_authorization_header("secure-http"))
|
||||
|
||||
assert header == "Bearer token-123"
|
||||
token = manager._states["secure-http"].token
|
||||
assert token is not None
|
||||
assert token.expires_at == oauth_now + timedelta(hours=1)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("offset", [-1, 0, 1], ids=["below-limit", "at-limit", "above-limit"])
|
||||
def test_expires_in_at_datetime_limit(monkeypatch, oauth_now, offset):
|
||||
# Integer division preserves the last whole second that can be added,
|
||||
# without the rounding in timedelta.total_seconds().
|
||||
ceiling = (datetime.max.replace(tzinfo=UTC) - oauth_now) // timedelta(seconds=1)
|
||||
expires_in = ceiling + offset
|
||||
_token_endpoint_returns(monkeypatch, {"access_token": "token-123", "expires_in": expires_in})
|
||||
manager = OAuthTokenManager.from_extensions_config(_oauth_server_config())
|
||||
|
||||
header = asyncio.run(manager.get_authorization_header("secure-http"))
|
||||
|
||||
assert header == "Bearer token-123"
|
||||
token = manager._states["secure-http"].token
|
||||
assert token is not None
|
||||
expected_lifetime = 3600 if offset > 0 else expires_in
|
||||
assert token.expires_at == oauth_now + timedelta(seconds=expected_lifetime)
|
||||
|
||||
|
||||
def test_oauth_extra_token_params_cannot_override_grant_type(monkeypatch):
|
||||
post_calls: list[dict[str, Any]] = []
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user