From fe6f1a32d85f9bccf4e5ca1f973d3e37a8e6cf78 Mon Sep 17 00:00:00 2001 From: zkasuran Date: Wed, 12 Aug 2026 18:06:56 +0530 Subject: [PATCH 1/2] fix(core): bound the MCP request timeout before arming setTimeout The public callMcpTool timeoutSeconds override and a caller-built config.timeoutSeconds reached setTimeout without an upper bound, so a value above 2147483 seconds or a non-finite one collapsed the delay to 1ms and aborted the request at once. Clamp both timer sites through one helper: cap oversized values at the setTimeout ceiling and fall back to the session timeout for unusable ones. This is the cap the CLI already applies to --timeout-seconds, now applied in the transport too. --- .changeset/bound-mcp-request-timeout.md | 10 +++ packages/core/lib/mcp-client.js | 22 ++++- packages/core/test/core.test.js | 102 ++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 .changeset/bound-mcp-request-timeout.md diff --git a/.changeset/bound-mcp-request-timeout.md b/.changeset/bound-mcp-request-timeout.md new file mode 100644 index 0000000..8ab2358 --- /dev/null +++ b/.changeset/bound-mcp-request-timeout.md @@ -0,0 +1,10 @@ +--- +"@call-e/core": patch +--- + +Bound the MCP request timeout on both ends before arming setTimeout. A `callMcpTool` +`timeoutSeconds` override or a `config.timeoutSeconds` above 2147483 seconds used to reach +`setTimeout` unchanged, where Node collapses any delay past 2147483647ms to 1ms and aborts +the request almost immediately. Oversized values are now capped at that ceiling. Non-finite +or non-positive values fall back to the session timeout. This matches the cap 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..91a25f7 100644 --- a/packages/core/lib/mcp-client.js +++ b/packages/core/lib/mcp-client.js @@ -2,10 +2,28 @@ 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. The CLI already caps --timeout-seconds at 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 that bound, +// so clamp it here rather than trusting the value. +const MIN_TIMEOUT_MS = 1000; +const MAX_TIMEOUT_MS = 2_147_483_647; + +function boundedTimeoutMs(seconds, fallbackMs) { + const requestedMs = Math.ceil(Number(seconds) * 1000); + if (!Number.isFinite(requestedMs) || requestedMs <= 0) { + return fallbackMs; + } + return Math.min(Math.max(requestedMs, MIN_TIMEOUT_MS), MAX_TIMEOUT_MS); +} + export class AuthRequiredError extends Error { constructor(message = "A usable CALL-E auth token is required.") { super(message); @@ -142,7 +160,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 +231,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..67e423a 100644 --- a/packages/core/test/core.test.js +++ b/packages/core/test/core.test.js @@ -488,3 +488,105 @@ 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}`); + }; +} +// APPEND-TIMEOUT-TESTS-HERE + +test("MCP client clamps an oversized tool-call timeout so the request is not aborted immediately", async () => { + const config = mcpConfig(makeTempRoot("calle-core-timeout-cap")); + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + callMcpTool({ + config, + toolName: "plan_call", + toolArguments: { goal: "Confirm the appointment" }, + timeoutSeconds: 5_000_000, + fetchImpl: recordingMcpFetch(delays, seen), + }), + ); + // 5_000_000s is 5e9ms, past the 2_147_483_647ms setTimeout ceiling. Unclamped it would + // collapse to a 1ms abort; clamped it stays at the ceiling. + assert.equal(seen["tools/call"], 2_147_483_647); +}); + +test("MCP client falls back to the session timeout when a tool-call override is unusable", async () => { + const config = { ...mcpConfig(makeTempRoot("calle-core-timeout-nan")), timeoutSeconds: 20 }; + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + callMcpTool({ + config, + toolName: "plan_call", + toolArguments: { goal: "Confirm the appointment" }, + timeoutSeconds: Number.NaN, + fetchImpl: recordingMcpFetch(delays, seen), + }), + ); + // A non-finite override used to reach setTimeout as NaN, which fires at 1ms. It now + // falls back to the 20s session timeout the handshake computed. + assert.equal(seen["tools/call"], 20_000); +}); +// APPEND-TIMEOUT-TESTS-2 + +test("MCP client clamps an oversized session timeout from config", async () => { + const config = { ...mcpConfig(makeTempRoot("calle-core-session-cap")), timeoutSeconds: 5_000_000 }; + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + listMcpTools({ config, fetchImpl: recordingMcpFetch(delays, seen) }), + ); + // openMcpSession derives the shared session timeout, so the handshake and the list call + // both stay at the ceiling instead of collapsing to a 1ms abort. + assert.equal(seen["initialize"], 2_147_483_647); + assert.equal(seen["tools/list"], 2_147_483_647); +}); + +test("MCP client keeps an in-range tool-call timeout unchanged", async () => { + const config = mcpConfig(makeTempRoot("calle-core-timeout-ok")); + const delays = []; + const seen = {}; + await withTimeoutDelayCapture(delays, () => + callMcpTool({ + config, + toolName: "plan_call", + toolArguments: { goal: "Confirm the appointment" }, + timeoutSeconds: 150, + fetchImpl: recordingMcpFetch(delays, seen), + }), + ); + assert.equal(seen["tools/call"], 150_000); +}); From cbb7eea8081610c04f671c5b9e4c55c85bc4d8dd Mon Sep 17 00:00:00 2001 From: zkasuran Date: Thu, 13 Aug 2026 08:43:14 +0530 Subject: [PATCH 2/2] fix(core): fall back to a safe timeout instead of capping an oversized one Validate the requested seconds before arming setTimeout: finite, positive and no greater than MAX_TIMER_SECONDS (2147483). An invalid config.timeoutSeconds now falls back to the default timeout and an invalid per-call override falls back to the session timeout, rather than capping an oversized value at the setTimeout ceiling. Capping turned a malformed value into a 24.8-day timeout that pinned the request and socket open instead of failing. Remove the APPEND-TIMEOUT-TESTS marker comments and cover the oversized, non-finite and negative cases on both timer sites. --- .changeset/bound-mcp-request-timeout.md | 16 ++-- packages/core/lib/mcp-client.js | 20 +++-- packages/core/test/core.test.js | 109 ++++++++++++++---------- 3 files changed, 85 insertions(+), 60 deletions(-) diff --git a/.changeset/bound-mcp-request-timeout.md b/.changeset/bound-mcp-request-timeout.md index 8ab2358..a9f047d 100644 --- a/.changeset/bound-mcp-request-timeout.md +++ b/.changeset/bound-mcp-request-timeout.md @@ -2,9 +2,13 @@ "@call-e/core": patch --- -Bound the MCP request timeout on both ends before arming setTimeout. A `callMcpTool` -`timeoutSeconds` override or a `config.timeoutSeconds` above 2147483 seconds used to reach -`setTimeout` unchanged, where Node collapses any delay past 2147483647ms to 1ms and aborts -the request almost immediately. Oversized values are now capped at that ceiling. Non-finite -or non-positive values fall back to the session timeout. This matches the cap the CLI -already applies to `--timeout-seconds`. +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 91a25f7..95b5eba 100644 --- a/packages/core/lib/mcp-client.js +++ b/packages/core/lib/mcp-client.js @@ -9,19 +9,23 @@ import { // 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. The CLI already caps --timeout-seconds at 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 that bound, -// so clamp it here rather than trusting the value. +// 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_TIMEOUT_MS = 2_147_483_647; +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 requestedMs = Math.ceil(Number(seconds) * 1000); - if (!Number.isFinite(requestedMs) || requestedMs <= 0) { + const parsed = Number(seconds); + if (!Number.isFinite(parsed) || parsed <= 0 || parsed > MAX_TIMER_SECONDS) { return fallbackMs; } - return Math.min(Math.max(requestedMs, MIN_TIMEOUT_MS), MAX_TIMEOUT_MS); + return Math.max(Math.ceil(parsed * 1000), MIN_TIMEOUT_MS); } export class AuthRequiredError extends Error { diff --git a/packages/core/test/core.test.js b/packages/core/test/core.test.js index 67e423a..360c92b 100644 --- a/packages/core/test/core.test.js +++ b/packages/core/test/core.test.js @@ -523,10 +523,14 @@ function recordingMcpFetch(delays, seen) { throw new Error(`Unexpected MCP method ${payload.method}`); }; } -// APPEND-TIMEOUT-TESTS-HERE - -test("MCP client clamps an oversized tool-call timeout so the request is not aborted immediately", async () => { - const config = mcpConfig(makeTempRoot("calle-core-timeout-cap")); +// 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, () => @@ -534,59 +538,72 @@ test("MCP client clamps an oversized tool-call timeout so the request is not abo config, toolName: "plan_call", toolArguments: { goal: "Confirm the appointment" }, - timeoutSeconds: 5_000_000, + timeoutSeconds: overrideSeconds, fetchImpl: recordingMcpFetch(delays, seen), }), ); - // 5_000_000s is 5e9ms, past the 2_147_483_647ms setTimeout ceiling. Unclamped it would - // collapse to a 1ms abort; clamped it stays at the ceiling. - assert.equal(seen["tools/call"], 2_147_483_647); -}); + return seen["tools/call"]; +} -test("MCP client falls back to the session timeout when a tool-call override is unusable", async () => { - const config = { ...mcpConfig(makeTempRoot("calle-core-timeout-nan")), timeoutSeconds: 20 }; +async function sessionDelays(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: Number.NaN, - fetchImpl: recordingMcpFetch(delays, seen), - }), + listMcpTools({ config, fetchImpl: recordingMcpFetch(delays, seen) }), ); - // A non-finite override used to reach setTimeout as NaN, which fires at 1ms. It now - // falls back to the 20s session timeout the handshake computed. - assert.equal(seen["tools/call"], 20_000); + 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); }); -// APPEND-TIMEOUT-TESTS-2 -test("MCP client clamps an oversized session timeout from config", async () => { - const config = { ...mcpConfig(makeTempRoot("calle-core-session-cap")), timeoutSeconds: 5_000_000 }; - const delays = []; - const seen = {}; - await withTimeoutDelayCapture(delays, () => - listMcpTools({ config, fetchImpl: recordingMcpFetch(delays, seen) }), - ); - // openMcpSession derives the shared session timeout, so the handshake and the list call - // both stay at the ceiling instead of collapsing to a 1ms abort. - assert.equal(seen["initialize"], 2_147_483_647); - assert.equal(seen["tools/list"], 2_147_483_647); +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 () => { - const config = mcpConfig(makeTempRoot("calle-core-timeout-ok")); - const delays = []; - const seen = {}; - await withTimeoutDelayCapture(delays, () => - callMcpTool({ - config, - toolName: "plan_call", - toolArguments: { goal: "Confirm the appointment" }, - timeoutSeconds: 150, - fetchImpl: recordingMcpFetch(delays, seen), - }), - ); - assert.equal(seen["tools/call"], 150_000); + 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); });