fix(core): bound the MCP request timeout before arming setTimeout

.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`.

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({

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);
+});