diff --git a/packages/normalization-core/src/number-coercion.test.ts b/packages/normalization-core/src/number-coercion.test.ts index 453156ade5f7..0ea660c47b51 100644 --- a/packages/normalization-core/src/number-coercion.test.ts +++ b/packages/normalization-core/src/number-coercion.test.ts @@ -95,6 +95,7 @@ describe("number-coercion", () => { expect(finiteSecondsToTimerSafeMilliseconds(1.5)).toBe(1_500); expect(finiteSecondsToTimerSafeMilliseconds(1.5, { floorSeconds: true })).toBe(1_000); expect(finiteSecondsToTimerSafeMilliseconds(10_000_000)).toBe(MAX_TIMER_TIMEOUT_MS); + expect(finiteSecondsToTimerSafeMilliseconds(Number.MAX_VALUE)).toBe(MAX_TIMER_TIMEOUT_MS); expect(finiteSecondsToTimerSafeMilliseconds("10")).toBeUndefined(); expect(finiteSecondsToTimerSafeMilliseconds(Number.POSITIVE_INFINITY)).toBeUndefined(); expect(clampTimerTimeoutMs(0, 10)).toBe(10); diff --git a/packages/normalization-core/src/number-coercion.ts b/packages/normalization-core/src/number-coercion.ts index 40de932f8883..dc55b26cb79f 100644 --- a/packages/normalization-core/src/number-coercion.ts +++ b/packages/normalization-core/src/number-coercion.ts @@ -214,6 +214,9 @@ export function finiteSecondsToTimerSafeMilliseconds( return undefined; } const boundedSeconds = opts.floorSeconds ? Math.floor(seconds) : seconds; + if (boundedSeconds >= MAX_TIMER_TIMEOUT_SECONDS) { + return MAX_TIMER_TIMEOUT_MS; + } const milliseconds = Math.floor(boundedSeconds * 1000); if (!Number.isFinite(milliseconds) || milliseconds <= 0) { return undefined; diff --git a/src/agents/mcp-transport-config.test.ts b/src/agents/mcp-transport-config.test.ts index d2ce99c774c6..e8f9b9f809e8 100644 --- a/src/agents/mcp-transport-config.test.ts +++ b/src/agents/mcp-transport-config.test.ts @@ -1,4 +1,5 @@ // Verifies MCP transport config normalization and startup-safety filtering. +import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { logWarn } from "../logger.js"; import { resolveMcpTransportConfig } from "./mcp-transport-config.js"; @@ -48,6 +49,32 @@ describe("resolveMcpTransportConfig", () => { ); }); + it("clamps oversized MCP request timeout aliases to the Node timer maximum", () => { + const seconds = resolveMcpTransportConfig("probe", { + command: "node", + connectTimeout: 1e306, + timeout: 1e306, + }); + const milliseconds = resolveMcpTransportConfig("probe", { + command: "node", + connectionTimeoutMs: 1e306, + requestTimeoutMs: 1e306, + }); + + expect(seconds).toEqual( + expect.objectContaining({ + connectionTimeoutMs: MAX_TIMER_TIMEOUT_MS, + requestTimeoutMs: MAX_TIMER_TIMEOUT_MS, + }), + ); + expect(milliseconds).toEqual( + expect.objectContaining({ + connectionTimeoutMs: MAX_TIMER_TIMEOUT_MS, + requestTimeoutMs: MAX_TIMER_TIMEOUT_MS, + }), + ); + }); + it("drops dangerous env overrides from stdio config", () => { // Stdio env is inherited executable process input. Block loader/shell hook // variables and child-process config pivots while preserving explicit MCP diff --git a/src/agents/mcp-transport-config.ts b/src/agents/mcp-transport-config.ts index 85870174dd78..e677343aef8a 100644 --- a/src/agents/mcp-transport-config.ts +++ b/src/agents/mcp-transport-config.ts @@ -1,6 +1,11 @@ /** * Resolves MCP transport command, environment, and timeout configuration. */ +import { + clampPositiveTimerTimeoutMs, + finiteSecondsToTimerSafeMilliseconds, + resolvePositiveTimerTimeoutMs, +} from "@openclaw/normalization-core/number-coercion"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js"; import { resolveOpenClawMcpTransportAlias } from "../config/mcp-config-normalize.js"; @@ -69,25 +74,28 @@ function getPositiveNumber(rawServer: unknown, keys: readonly string[]): number function getConnectionTimeoutMs(rawServer: unknown): number { const milliseconds = getPositiveNumber(rawServer, ["connectionTimeoutMs"]); if (milliseconds) { - return Math.floor(milliseconds); + return clampPositiveTimerTimeoutMs(milliseconds) ?? DEFAULT_CONNECTION_TIMEOUT_MS; } const seconds = getPositiveNumber(rawServer, ["connectTimeout", "connect_timeout"]); if (seconds) { - return Math.floor(seconds * 1_000); + return finiteSecondsToTimerSafeMilliseconds(seconds) ?? 1; } return DEFAULT_CONNECTION_TIMEOUT_MS; } -function getRequestTimeoutMs(rawServer: unknown): number { +export function resolveMcpRequestTimeoutMs( + rawServer: unknown, + fallbackMs = DEFAULT_REQUEST_TIMEOUT_MS, +): number { const milliseconds = getPositiveNumber(rawServer, ["requestTimeoutMs"]); if (milliseconds) { - return Math.floor(milliseconds); + return clampPositiveTimerTimeoutMs(milliseconds) ?? DEFAULT_REQUEST_TIMEOUT_MS; } const seconds = getPositiveNumber(rawServer, ["timeout"]); if (seconds) { - return Math.floor(seconds * 1_000); + return finiteSecondsToTimerSafeMilliseconds(seconds) ?? 1; } - return DEFAULT_REQUEST_TIMEOUT_MS; + return resolvePositiveTimerTimeoutMs(fallbackMs, DEFAULT_REQUEST_TIMEOUT_MS); } function getBooleanField(rawServer: unknown, keys: readonly string[]): boolean | undefined { @@ -182,7 +190,7 @@ function resolveHttpTransportConfig( : {}), description: describeHttpMcpServerLaunchConfig(launch.config), connectionTimeoutMs: getConnectionTimeoutMs(rawServer), - requestTimeoutMs: getRequestTimeoutMs(rawServer), + requestTimeoutMs: resolveMcpRequestTimeoutMs(rawServer), supportsParallelToolCalls: getBooleanField(rawServer, ["supportsParallelToolCalls", "supports_parallel_tool_calls"]) ?? false, @@ -217,7 +225,7 @@ export function resolveMcpTransportConfig( cwd: stdioLaunch.config.cwd, description: describeStdioMcpServerLaunchConfig(stdioLaunch.config), connectionTimeoutMs: getConnectionTimeoutMs(rawServer), - requestTimeoutMs: getRequestTimeoutMs(rawServer), + requestTimeoutMs: resolveMcpRequestTimeoutMs(rawServer), supportsParallelToolCalls: getBooleanField(rawServer, ["supportsParallelToolCalls", "supports_parallel_tool_calls"]) ?? false, diff --git a/src/node-host/mcp.test.ts b/src/node-host/mcp.test.ts index 1ee8f2810041..5433f392ff1b 100644 --- a/src/node-host/mcp.test.ts +++ b/src/node-host/mcp.test.ts @@ -2,6 +2,7 @@ import { ErrorCode, type CallToolResult, type Tool } from "@modelcontextprotocol/sdk/types.js"; import { expectDefined } from "@openclaw/normalization-core"; +import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; import { describe, expect, it, vi } from "vitest"; import { OpenClawSchema } from "../config/zod-schema.js"; import { startNodeHostMcpManager } from "./mcp.js"; @@ -241,6 +242,20 @@ describe("node host MCP manager", () => { await manager.close(); }); + it("clamps oversized configured and requested MCP tool timeouts", async () => { + const client = createClient({ tools: [tool("search")] }); + const manager = await startNodeHostMcpManager( + { docs: { command: "docs", timeout: 1e306 } }, + { createClient: () => client, resolveTransport: () => transport, warn: vi.fn() }, + ); + + await manager.callMcpTool({ server: "docs", tool: "search", timeoutMs: 1e306 }); + expect(client.callTool).toHaveBeenCalledWith({ name: "search", arguments: {} }, undefined, { + timeout: MAX_TIMER_TIMEOUT_MS, + }); + await manager.close(); + }); + it("bounds remote MCP error messages", async () => { const client = createClient({ tools: [tool("fail")], diff --git a/src/node-host/mcp.ts b/src/node-host/mcp.ts index 5035c294750b..a60c2a4544a7 100644 --- a/src/node-host/mcp.ts +++ b/src/node-host/mcp.ts @@ -3,12 +3,14 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import { ErrorCode, type CallToolResult, type Tool } from "@modelcontextprotocol/sdk/types.js"; import { redactSensitiveUrlLikeString } from "@openclaw/net-policy/redact-sensitive-url"; +import { clampPositiveTimerTimeoutMs } from "@openclaw/normalization-core/number-coercion"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; import type { NodePluginToolDescriptor } from "../../packages/gateway-protocol/src/schema/nodes.js"; import { matchesMcpToolFilterPattern } from "../agents/agent-bundle-mcp-filter.js"; import { createMcpJsonSchemaValidator } from "../agents/mcp-json-schema-validator.js"; import { sanitizeMcpMetadataText } from "../agents/mcp-metadata.js"; +import { resolveMcpRequestTimeoutMs } from "../agents/mcp-transport-config.js"; import { resolveMcpTransport } from "../agents/mcp-transport.js"; import { normalizeConfiguredMcpServers } from "../config/mcp-config-normalize.js"; import type { McpServerConfig } from "../config/types.mcp.js"; @@ -273,23 +275,7 @@ async function listAllTools( } function resolveCallTimeoutMs(value: number | undefined): number { - return typeof value === "number" && Number.isFinite(value) && value > 0 - ? Math.floor(value) - : NODE_MCP_TOOL_CALL_TIMEOUT_MS; -} - -function resolveServerToolCallTimeoutMs(config: McpServerConfig): number { - if ( - typeof config.requestTimeoutMs === "number" && - Number.isFinite(config.requestTimeoutMs) && - config.requestTimeoutMs > 0 - ) { - return Math.floor(config.requestTimeoutMs); - } - if (typeof config.timeout === "number" && Number.isFinite(config.timeout) && config.timeout > 0) { - return Math.floor(config.timeout * 1_000); - } - return NODE_MCP_TOOL_CALL_TIMEOUT_MS; + return clampPositiveTimerTimeoutMs(value) ?? NODE_MCP_TOOL_CALL_TIMEOUT_MS; } function isMcpTimeoutError(error: unknown): boolean { @@ -339,7 +325,7 @@ export async function startNodeHostMcpManager( client, connected: false, tools: new Set(), - toolCallTimeoutMs: resolveServerToolCallTimeoutMs(config), + toolCallTimeoutMs: resolveMcpRequestTimeoutMs(config, NODE_MCP_TOOL_CALL_TIMEOUT_MS), detachStderr: resolved.detachStderr, }; // MCP Client exposes callback properties rather than an EventTarget surface.