fix(mattermost): add timeout to REST client requests (#102027)

* fix(mattermost): add timeout to REST client requests

* fixup: preserve Mattermost DM retry timeout

* test(mattermost): reuse hanging server helper

* test(mattermost): satisfy timeout lint

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>

extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts

@@ -0,0 +1,123 @@
+// Mattermost tests cover real REST client timeout behavior.
+import { withServer } from "openclaw/plugin-sdk/test-env";
+import { describe, expect, it } from "vitest";
+import {
+  createMattermostClient,
+  createMattermostDirectChannelWithRetry,
+  fetchMattermostMe,
+} from "./client.js";
+
+type OperationOutcome =
+  | { status: "resolved" }
+  | { status: "rejected"; error: unknown }
+  | {
+      status: "pending";
+    };
+
+function delay(ms: number): Promise<void> {
+  return new Promise((resolve) => {
+    setTimeout(resolve, ms);
+  });
+}
+
+async function settleWithin(
+  promise: Promise<unknown>,
+  timeoutMs: number,
+): Promise<OperationOutcome> {
+  return await Promise.race([
+    promise.then(
+      () => ({ status: "resolved" as const }),
+      (error: unknown) => ({ status: "rejected" as const, error }),
+    ),
+    delay(timeoutMs).then(() => ({ status: "pending" as const })),
+  ]);
+}
+
+async function expectTimeoutRejection(promise: Promise<unknown>, timeoutMs: number): Promise<void> {
+  const outcome = await settleWithin(promise, timeoutMs);
+  expect(outcome.status).toBe("rejected");
+  if (outcome.status !== "rejected") {
+    throw new Error(`expected timeout rejection, got ${outcome.status}`);
+  }
+  expect(outcome.error).toBeInstanceOf(Error);
+  expect(outcome.error instanceof Error ? outcome.error.name : "").toMatch(
+    /^(AbortError|TimeoutError)$/,
+  );
+}
+
+async function withHangingMattermostServer(
+  run: (server: {
+    baseUrl: string;
+    received: Promise<void>;
+    requestCount: () => number;
+  }) => Promise<void>,
+): Promise<void> {
+  let requestCount = 0;
+  let notifyRequest: () => void = () => {};
+  const received = new Promise<void>((resolve) => {
+    notifyRequest = resolve;
+  });
+  await withServer(
+    (request) => {
+      requestCount += 1;
+      notifyRequest();
+      request.resume();
+    },
+    async (baseUrl) => run({ baseUrl, received, requestCount: () => requestCount }),
+  );
+}
+
+describe("Mattermost REST client fetch timeout", () => {
+  it("rejects a hanging real loopback request at the configured client timeout", async () => {
+    await withHangingMattermostServer(async (server) => {
+      const client = createMattermostClient({
+        baseUrl: server.baseUrl,
+        botToken: "bot-token",
+        allowPrivateNetwork: true,
+        timeoutMs: 50,
+      });
+      const request = fetchMattermostMe(client);
+
+      await server.received;
+      expect(server.requestCount()).toBe(1);
+      await expectTimeoutRejection(request, 750);
+    });
+  });
+
+  it("preserves a caller AbortSignal while applying the default request timeout", async () => {
+    await withHangingMattermostServer(async (server) => {
+      const client = createMattermostClient({
+        baseUrl: server.baseUrl,
+        botToken: "bot-token",
+        allowPrivateNetwork: true,
+        timeoutMs: 30_000,
+      });
+      const controller = new AbortController();
+      const request = client.request("/users/me", { signal: controller.signal });
+
+      await server.received;
+      controller.abort();
+      await expectTimeoutRejection(request, 750);
+    });
+  });
+
+  it("preserves configured DM retry timeouts longer than the client default", async () => {
+    await withHangingMattermostServer(async (server) => {
+      const client = createMattermostClient({
+        baseUrl: server.baseUrl,
+        botToken: "bot-token",
+        allowPrivateNetwork: true,
+        timeoutMs: 50,
+      });
+      const request = createMattermostDirectChannelWithRetry(client, ["bot-user", "dm-user"], {
+        maxRetries: 0,
+        timeoutMs: 250,
+      });
+      request.catch(() => undefined);
+
+      await server.received;
+      expect(await settleWithin(request, 120)).toEqual({ status: "pending" });
+      await expectTimeoutRejection(request, 600);
+    });
+  });
+});

extensions/mattermost/src/mattermost/client.ts

@@ -1,4 +1,5 @@
 // Mattermost plugin module implements client behavior.
+import { buildTimeoutAbortSignal } from "openclaw/plugin-sdk/extension-shared";
 import { resolveTimerTimeoutMs } from "openclaw/plugin-sdk/number-runtime";
 import {
   readProviderJsonResponse,
@@ -17,6 +18,7 @@ import {
 import { z } from "zod";
 
 const MATTERMOST_ERROR_BODY_LIMIT_BYTES = 8 * 1024;
+const MATTERMOST_REQUEST_TIMEOUT_MS = 30_000;
 // Mattermost REST control-plane JSON (posts, users, channels, file-upload
 // results) stays well under a megabyte; cap successful JSON the same way the
 // shared provider path is capped so an untrusted/self-hosted homeserver cannot
@@ -27,12 +29,15 @@ const MATTERMOST_TEXT_RESPONSE_LIMIT_BYTES = 64 * 1024;
 const NULL_BODY_STATUSES = new Set([101, 204, 205, 304]);
 
 export type MattermostFetch = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
+export type MattermostRequestInit = RequestInit & {
+  timeoutMs?: number;
+};
 
 export type MattermostClient = {
   baseUrl: string;
   apiBaseUrl: string;
   token: string;
-  request: <T>(path: string, init?: RequestInit) => Promise<T>;
+  request: <T>(path: string, init?: MattermostRequestInit) => Promise<T>;
   /** Guarded fetch implementation; use in place of raw fetch for outbound requests. */
   fetchImpl: MattermostFetch;
 };
@@ -174,6 +179,8 @@ export function createMattermostClient(params: {
   baseUrl: string;
   botToken: string;
   fetchImpl?: MattermostFetch;
+  /** Timeout for REST requests in milliseconds (default: 30000). */
+  timeoutMs?: number;
   /** Allow requests to private/internal IPs (self-hosted/LAN deployments). */
   allowPrivateNetwork?: boolean;
 }): MattermostClient {
@@ -183,26 +190,56 @@ export function createMattermostClient(params: {
   }
   const apiBaseUrl = `${baseUrl}/api/v4`;
   const token = params.botToken.trim();
+  const requestTimeoutMs = resolveTimerTimeoutMs(params.timeoutMs, MATTERMOST_REQUEST_TIMEOUT_MS);
   // When no custom fetchImpl is provided (production path), use an SSRF-guarded wrapper
   // that validates the target URL before making the request (DNS rebinding protection etc.).
   // A custom fetchImpl is accepted for testing and special cases.
   const externalFetchImpl = params.fetchImpl;
 
-  const guardedFetchImpl: MattermostFetch = async (input, init) => {
+  const guardedFetchImpl = async (
+    input: RequestInfo | URL,
+    init?: MattermostRequestInit,
+  ): Promise<Response> => {
     const url =
       typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
+    const { timeoutMs: initTimeoutMs, ...requestInit } = init ?? {};
+    const timeoutMs = resolveTimerTimeoutMs(initTimeoutMs, requestTimeoutMs);
     const { response, release } = await fetchWithSsrFGuard({
       url,
-      init,
+      init: requestInit,
       auditContext: "mattermost-api",
       policy: ssrfPolicyFromPrivateNetworkOptIn(params.allowPrivateNetwork),
+      signal: requestInit.signal ?? undefined,
+      timeoutMs,
     });
     return responseWithRelease(response, release);
   };
 
-  const fetchImpl = externalFetchImpl ?? guardedFetchImpl;
+  const timedExternalFetchImpl:
+    | ((input: RequestInfo | URL, init?: MattermostRequestInit) => Promise<Response>)
+    | undefined = externalFetchImpl
+    ? async (input, init) => {
+        const url =
+          typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
+        const { timeoutMs: initTimeoutMs, ...requestInit } = init ?? {};
+        const timeoutMs = resolveTimerTimeoutMs(initTimeoutMs, requestTimeoutMs);
+        const { signal, cleanup } = buildTimeoutAbortSignal({
+          timeoutMs,
+          signal: requestInit.signal ?? undefined,
+          operation: "mattermost-api",
+          url,
+        });
+        try {
+          return await externalFetchImpl(input, { ...requestInit, signal });
+        } finally {
+          cleanup();
+        }
+      }
+    : undefined;
 
-  const request = async <T>(path: string, init?: RequestInit): Promise<T> => {
+  const fetchImpl = timedExternalFetchImpl ?? guardedFetchImpl;
+
+  const request = async <T>(path: string, init?: MattermostRequestInit): Promise<T> => {
     const url = buildMattermostApiUrl(baseUrl, path);
     const headers = new Headers(init?.headers);
     headers.set("Authorization", `Bearer ${token}`);
@@ -287,11 +324,13 @@ export async function createMattermostDirectChannel(
   client: MattermostClient,
   userIds: string[],
   signal?: AbortSignal,
+  timeoutMs?: number,
 ): Promise<MattermostChannel> {
   return await client.request<MattermostChannel>("/channels/direct", {
     method: "POST",
     body: JSON.stringify(userIds),
     signal,
+    timeoutMs,
   });
 }
 
@@ -406,7 +445,12 @@ export async function createMattermostDirectChannelWithRetry(
       const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
 
       try {
-        const result = await createMattermostDirectChannel(client, userIds, controller.signal);
+        const result = await createMattermostDirectChannel(
+          client,
+          userIds,
+          controller.signal,
+          timeoutMs,
+        );
         return result;
       } finally {
         clearTimeout(timeoutId);

src/plugin-sdk/test-helpers/http-test-server.ts

@@ -15,8 +15,11 @@ export async function withServer(handler: RequestListener, fn: (baseUrl: string)
   try {
     await fn(`http://127.0.0.1:${address.port}`);
   } finally {
-    await new Promise<void>((resolve) => {
+    const closed = new Promise<void>((resolve) => {
       server.close(() => resolve());
     });
+    // Hanging-response tests must still release active sockets when their assertion fails.
+    server.closeAllConnections();
+    await closed;
   }
 }