diff --git a/.changeset/bound-mcp-request-timeout.md b/.changeset/bound-mcp-request-timeout.md new file mode 100644 index 0000000..a9f047d --- /dev/null +++ b/.changeset/bound-mcp-request-timeout.md @@ -0,0 +1,14 @@ +--- +"@call-e/core": patch +--- + +Validate the MCP request timeout before arming setTimeout and fall back to a safe value +rather than capping. A `callMcpTool` `timeoutSeconds` override or a `config.timeoutSeconds` +reaches Node's timer, where any delay past 2147483647ms (~24.8 days) collapses to 1ms and +aborts the request almost immediately. The seconds value is now required to be finite, +positive and no greater than 2147483 (the largest whole-second delay the timer keeps intact). +An out-of-range or non-finite session value falls back to the 15s default; an out-of-range or +non-finite per-call override falls back to the already computed session timeout. Capping an +oversized value at the ceiling was worse than the abort it replaced: it turned a malformed +value into a ~24.8-day timeout that held the request, socket and caller resources open instead +of failing. This matches the validation the CLI already applies to `--timeout-seconds`. diff --git a/packages/core/lib/mcp-client.js b/packages/core/lib/mcp-client.js index 43e5c5a..95b5eba 100644 --- a/packages/core/lib/mcp-client.js +++ b/packages/core/lib/mcp-client.js @@ -2,10 +2,32 @@ import { readJson, tokenCachePath, tokenIsUsable } from "./cache.js"; import { DEFAULT_MCP_CLIENT_NAME, DEFAULT_MCP_CLIENT_VERSION, + DEFAULT_TIMEOUT_SECONDS, INTEGRATION_HEADER, MCP_PROTOCOL_VERSION, } from "./constants.js"; +// setTimeout collapses any delay above 2_147_483_647ms (~24.8 days) to 1ms, so a very +// large timeout fires the abort almost at once and cancels the request the caller meant +// to keep waiting on. Capping an oversized value at that ceiling instead would be worse: +// it turns a malformed or attacker-influenced timeout into a ~24.8-day one that holds the +// request, socket, AbortController and caller resources open rather than failing. The CLI +// validates --timeout-seconds against this ceiling (see packages/cli/lib/config.js), but +// the public callMcpTool timeoutSeconds override and a caller-built config.timeoutSeconds +// reach the timer arithmetic below without it, so validate the seconds here and fall back +// to a safe timeout rather than trusting or capping the value. +const MIN_TIMEOUT_MS = 1000; +const MAX_TIMER_DELAY_MS = 2_147_483_647; +const MAX_TIMER_SECONDS = Math.floor(MAX_TIMER_DELAY_MS / 1000); + +function boundedTimeoutMs(seconds, fallbackMs) { + const parsed = Number(seconds); + if (!Number.isFinite(parsed) || parsed <= 0 || parsed > MAX_TIMER_SECONDS) { + return fallbackMs; + } + return Math.max(Math.ceil(parsed * 1000), MIN_TIMEOUT_MS); +} + export class AuthRequiredError extends Error { constructor(message = "A usable CALL-E auth token is required.") { super(message); @@ -142,7 +164,7 @@ function accessTokenFromCache(config) { async function openMcpSession({ config, fetchImpl }) { requireFetch(fetchImpl); const accessToken = accessTokenFromCache(config); - const timeoutMs = Math.max(Math.ceil(Number(config.timeoutSeconds || 15) * 1000), 1000); + const timeoutMs = boundedTimeoutMs(config.timeoutSeconds, DEFAULT_TIMEOUT_SECONDS * 1000); const commonHeaders = { Accept: "application/json, text/event-stream", "Content-Type": "application/json", @@ -213,7 +235,7 @@ export async function callMcpTool({ } const toolCallTimeoutMs = timeoutSeconds === null ? timeoutMs - : Math.max(Math.ceil(Number(timeoutSeconds) * 1000), 1000); + : boundedTimeoutMs(timeoutSeconds, timeoutMs); const response = await requestJsonRpc(fetchImpl, config.serverUrl, { headers: rpcHeaders, payload: buildJsonRpcPayload({ diff --git a/packages/core/test/core.test.js b/packages/core/test/core.test.js index 540ebaf..360c92b 100644 --- a/packages/core/test/core.test.js +++ b/packages/core/test/core.test.js @@ -488,3 +488,122 @@ test("MCP client reports request timeouts", async () => { }, ); }); + +// The delay handed to setTimeout is decided just before each request is sent, so a stub +// that records delays.at(-1) per method reads the exact timer armed for that request. +async function withTimeoutDelayCapture(delays, run) { + const realSetTimeout = globalThis.setTimeout; + globalThis.setTimeout = (handler, delay, ...rest) => { + delays.push(delay); + return realSetTimeout(handler, delay, ...rest); + }; + try { + await run(); + } finally { + globalThis.setTimeout = realSetTimeout; + } +} + +function recordingMcpFetch(delays, seen) { + return async (_url, init) => { + const payload = JSON.parse(init.body); + seen[payload.method] = delays.at(-1); + if (payload.method === "initialize") { + return jsonResponse({ result: {} }, { headers: { "mcp-session-id": "mcp-session-timeout" } }); + } + if (payload.method === "notifications/initialized") { + return jsonResponse({}); + } + if (payload.method === "tools/list") { + return jsonResponse({ result: { tools: [] } }); + } + if (payload.method === "tools/call") { + return jsonResponse({ result: { content: [{ type: "text", text: "ok" }] } }); + } + throw new Error(`Unexpected MCP method ${payload.method}`); + }; +} +// Node's setTimeout collapses any delay above 2_147_483_647ms (~24.8 days) to 1ms, so the +// core validates the requested seconds against this ceiling and falls back to a safe timeout +// rather than arming the timer with a disabled or immediate-abort value. MAX_TIMER_SECONDS +// is the largest whole-second override the timer keeps intact, matching mcp-client.js. +const MAX_TIMER_SECONDS = Math.floor(2_147_483_647 / 1000); + +async function toolCallDelay(overrideSeconds, sessionTimeoutSeconds, rootName) { + const config = { ...mcpConfig(makeTempRoot(rootName)), timeoutSeconds: sessionTimeoutSeconds }; + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + callMcpTool({ + config, + toolName: "plan_call", + toolArguments: { goal: "Confirm the appointment" }, + timeoutSeconds: overrideSeconds, + fetchImpl: recordingMcpFetch(delays, seen), + }), + ); + return seen["tools/call"]; +} + +async function sessionDelays(sessionTimeoutSeconds, rootName) { + const config = { ...mcpConfig(makeTempRoot(rootName)), timeoutSeconds: sessionTimeoutSeconds }; + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + listMcpTools({ config, fetchImpl: recordingMcpFetch(delays, seen) }), + ); + return seen; +} + +test("MCP client falls back to the session timeout for an oversized tool-call override", async () => { + // One second past the ceiling is invalid, not a 24.8-day timeout, so the request keeps the + // 20s session timeout the handshake computed. + assert.equal(await toolCallDelay(MAX_TIMER_SECONDS + 1, 20, "calle-core-tool-over"), 20_000); +}); + +test("MCP client falls back to the session timeout for a non-finite tool-call override", async () => { + assert.equal(await toolCallDelay(Number.POSITIVE_INFINITY, 20, "calle-core-tool-inf"), 20_000); +}); + +test("MCP client falls back to the session timeout for a NaN tool-call override", async () => { + assert.equal(await toolCallDelay(Number.NaN, 20, "calle-core-tool-nan"), 20_000); +}); + +test("MCP client falls back to the session timeout for a negative tool-call override", async () => { + assert.equal(await toolCallDelay(-30, 20, "calle-core-tool-neg"), 20_000); +}); + +test("MCP client accepts a tool-call override at the timer ceiling", async () => { + // The boundary value is the largest delay setTimeout keeps intact, so it passes through. + assert.equal(await toolCallDelay(MAX_TIMER_SECONDS, 20, "calle-core-tool-max"), MAX_TIMER_SECONDS * 1000); +}); + +test("MCP client keeps an in-range tool-call timeout unchanged", async () => { + assert.equal(await toolCallDelay(150, 15, "calle-core-tool-ok"), 150_000); +}); + +test("MCP client falls back to the default timeout for an oversized session timeout", async () => { + // openMcpSession derives the shared session timeout, so an out-of-range config value falls + // back to the 15s default for the handshake and the list call instead of disabling the timer. + const seen = await sessionDelays(MAX_TIMER_SECONDS + 1, "calle-core-session-over"); + assert.equal(seen["initialize"], 15_000); + assert.equal(seen["tools/list"], 15_000); +}); + +test("MCP client falls back to the default timeout for a non-finite session timeout", async () => { + const seen = await sessionDelays(Number.POSITIVE_INFINITY, "calle-core-session-inf"); + assert.equal(seen["initialize"], 15_000); + assert.equal(seen["tools/list"], 15_000); +}); + +test("MCP client falls back to the default timeout for a negative session timeout", async () => { + const seen = await sessionDelays(-5, "calle-core-session-neg"); + assert.equal(seen["initialize"], 15_000); + assert.equal(seen["tools/list"], 15_000); +}); + +test("MCP client accepts a session timeout at the timer ceiling", async () => { + const seen = await sessionDelays(MAX_TIMER_SECONDS, "calle-core-session-max"); + assert.equal(seen["initialize"], MAX_TIMER_SECONDS * 1000); + assert.equal(seen["tools/list"], MAX_TIMER_SECONDS * 1000); +});