fix(mattermost): allow reachable interaction callback URLs (#37543)

Merged via squash.

Prepared head SHA: 4d593731be
Co-authored-by: mukhtharcm <56378562+mukhtharcm@users.noreply.github.com>
Co-authored-by: mukhtharcm <56378562+mukhtharcm@users.noreply.github.com>
Reviewed-by: @mukhtharcm

extensions/mattermost/src/channel.ts

@@ -165,7 +165,10 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
     if (params.buttons && Array.isArray(params.buttons)) {
       const account = resolveMattermostAccount({ cfg, accountId: resolvedAccountId });
       if (account.botToken) setInteractionSecret(account.accountId, account.botToken);
-      const callbackUrl = resolveInteractionCallbackUrl(account.accountId, cfg);
+      const callbackUrl = resolveInteractionCallbackUrl(account.accountId, {
+        gateway: cfg.gateway,
+        interactions: account.config.interactions,
+      });
 
       // Flatten 2D array (rows of buttons) to 1D — core schema sends Array<Array<Button>>
       // but Mattermost doesn't have row layout, so we flatten all rows into a single list.

extensions/mattermost/src/mattermost/interactions.test.ts

@@ -1,11 +1,16 @@
-import { type IncomingMessage } from "node:http";
+import { type IncomingMessage, type ServerResponse } from "node:http";
 import { describe, expect, it, beforeEach, afterEach } from "vitest";
+import { setMattermostRuntime } from "../runtime.js";
+import { resolveMattermostAccount } from "./accounts.js";
+import type { MattermostClient } from "./client.js";
 import {
   buildButtonAttachments,
+  computeInteractionCallbackUrl,
+  createMattermostInteractionHandler,
   generateInteractionToken,
   getInteractionCallbackUrl,
   getInteractionSecret,
-  isLocalhostRequest,
+  resolveInteractionCallbackPath,
   resolveInteractionCallbackUrl,
   setInteractionCallbackUrl,
   setInteractionSecret,
@@ -132,7 +137,9 @@ describe("callback URL registry", () => {
 
 describe("resolveInteractionCallbackUrl", () => {
   afterEach(() => {
-    setInteractionCallbackUrl("resolve-test", "");
+    for (const accountId of ["cached", "default", "acct", "myaccount"]) {
+      setInteractionCallbackUrl(accountId, "");
+    }
   });
 
   it("prefers cached URL from registry", () => {
@@ -140,19 +147,99 @@ describe("resolveInteractionCallbackUrl", () => {
     expect(resolveInteractionCallbackUrl("cached")).toBe("http://cached:1234/path");
   });
 
-  it("falls back to computed URL from gateway port config", () => {
-    const url = resolveInteractionCallbackUrl("default", { gateway: { port: 9999 } });
+  it("recomputes from config when bypassing the cache explicitly", () => {
+    setInteractionCallbackUrl("acct", "http://cached:1234/path");
+    const url = computeInteractionCallbackUrl("acct", {
+      gateway: { port: 9999, customBindHost: "gateway.internal" },
+    });
+    expect(url).toBe("http://gateway.internal:9999/mattermost/interactions/acct");
+  });
+
+  it("uses interactions.callbackBaseUrl when configured", () => {
+    const url = resolveInteractionCallbackUrl("default", {
+      channels: {
+        mattermost: {
+          interactions: {
+            callbackBaseUrl: "https://gateway.example.com/openclaw",
+          },
+        },
+      },
+    });
+    expect(url).toBe("https://gateway.example.com/openclaw/mattermost/interactions/default");
+  });
+
+  it("trims trailing slashes from callbackBaseUrl", () => {
+    const url = resolveInteractionCallbackUrl("acct", {
+      channels: {
+        mattermost: {
+          interactions: {
+            callbackBaseUrl: "https://gateway.example.com/root///",
+          },
+        },
+      },
+    });
+    expect(url).toBe("https://gateway.example.com/root/mattermost/interactions/acct");
+  });
+
+  it("uses merged per-account interactions.callbackBaseUrl", () => {
+    const cfg = {
+      gateway: { port: 9999 },
+      channels: {
+        mattermost: {
+          accounts: {
+            acct: {
+              botToken: "bot-token",
+              baseUrl: "https://chat.example.com",
+              interactions: {
+                callbackBaseUrl: "https://gateway.example.com/root",
+              },
+            },
+          },
+        },
+      },
+    };
+    const account = resolveMattermostAccount({
+      cfg,
+      accountId: "acct",
+      allowUnresolvedSecretRef: true,
+    });
+    const url = resolveInteractionCallbackUrl(account.accountId, {
+      gateway: cfg.gateway,
+      interactions: account.config.interactions,
+    });
+    expect(url).toBe("https://gateway.example.com/root/mattermost/interactions/acct");
+  });
+
+  it("falls back to gateway.customBindHost when configured", () => {
+    const url = resolveInteractionCallbackUrl("default", {
+      gateway: { port: 9999, customBindHost: "gateway.internal" },
+    });
+    expect(url).toBe("http://gateway.internal:9999/mattermost/interactions/default");
+  });
+
+  it("falls back to localhost when customBindHost is a wildcard bind address", () => {
+    const url = resolveInteractionCallbackUrl("default", {
+      gateway: { port: 9999, customBindHost: "0.0.0.0" },
+    });
     expect(url).toBe("http://localhost:9999/mattermost/interactions/default");
   });
 
+  it("brackets IPv6 custom bind hosts", () => {
+    const url = resolveInteractionCallbackUrl("acct", {
+      gateway: { port: 9999, customBindHost: "::1" },
+    });
+    expect(url).toBe("http://[::1]:9999/mattermost/interactions/acct");
+  });
+
   it("uses default port 18789 when no config provided", () => {
     const url = resolveInteractionCallbackUrl("myaccount");
     expect(url).toBe("http://localhost:18789/mattermost/interactions/myaccount");
   });
+});
 
-  it("uses default port when gateway config has no port", () => {
-    const url = resolveInteractionCallbackUrl("acct", { gateway: {} });
-    expect(url).toBe("http://localhost:18789/mattermost/interactions/acct");
+describe("resolveInteractionCallbackPath", () => {
+  it("builds the per-account callback path", () => {
+    expect(resolveInteractionCallbackPath("acct")).toBe("/mattermost/interactions/acct");
   });
 });
 
@@ -299,37 +386,134 @@ describe("buildButtonAttachments", () => {
   });
 });
 
-// ── isLocalhostRequest ───────────────────────────────────────────────
+describe("createMattermostInteractionHandler", () => {
+  beforeEach(() => {
+    setMattermostRuntime({
+      system: {
+        enqueueSystemEvent: () => {},
+      },
+    } as unknown as Parameters<typeof setMattermostRuntime>[0]);
+    setInteractionSecret("acct", "bot-token");
+  });
 
-describe("isLocalhostRequest", () => {
-  function fakeReq(remoteAddress?: string): IncomingMessage {
-    return {
-      socket: { remoteAddress },
-    } as unknown as IncomingMessage;
+  function createReq(params: {
+    method?: string;
+    body?: unknown;
+    remoteAddress?: string;
+  }): IncomingMessage {
+    const body = params.body === undefined ? "" : JSON.stringify(params.body);
+    const listeners = new Map<string, Array<(...args: unknown[]) => void>>();
+
+    const req = {
+      method: params.method ?? "POST",
+      socket: { remoteAddress: params.remoteAddress ?? "203.0.113.10" },
+      on(event: string, handler: (...args: unknown[]) => void) {
+        const existing = listeners.get(event) ?? [];
+        existing.push(handler);
+        listeners.set(event, existing);
+        return this;
+      },
+    } as IncomingMessage & { emitTest: (event: string, ...args: unknown[]) => void };
+
+    req.emitTest = (event: string, ...args: unknown[]) => {
+      const handlers = listeners.get(event) ?? [];
+      for (const handler of handlers) {
+        handler(...args);
+      }
+    };
+
+    queueMicrotask(() => {
+      if (body) {
+        req.emitTest("data", Buffer.from(body));
+      }
+      req.emitTest("end");
+    });
+
+    return req;
   }
 
-  it("accepts 127.0.0.1", () => {
-    expect(isLocalhostRequest(fakeReq("127.0.0.1"))).toBe(true);
+  function createRes(): ServerResponse & { headers: Record<string, string>; body: string } {
+    const res = {
+      statusCode: 200,
+      headers: {} as Record<string, string>,
+      body: "",
+      setHeader(name: string, value: string) {
+        res.headers[name] = value;
+      },
+      end(chunk?: string) {
+        res.body = chunk ?? "";
+      },
+    };
+    return res as unknown as ServerResponse & { headers: Record<string, string>; body: string };
+  }
+
+  it("accepts non-localhost requests when the interaction token is valid", async () => {
+    const context = { action_id: "approve" };
+    const token = generateInteractionToken(context, "acct");
+    const requestLog: Array<{ path: string; method?: string }> = [];
+    const handler = createMattermostInteractionHandler({
+      client: {
+        request: async (path: string, init?: { method?: string }) => {
+          requestLog.push({ path, method: init?.method });
+          if (init?.method === "PUT") {
+            return { id: "post-1" };
+          }
+          return {
+            message: "Choose",
+            props: {
+              attachments: [{ actions: [{ id: "approve", name: "Approve" }] }],
+            },
+          };
+        },
+      } as unknown as MattermostClient,
+      botUserId: "bot",
+      accountId: "acct",
+    });
+
+    const req = createReq({
+      remoteAddress: "198.51.100.8",
+      body: {
+        user_id: "user-1",
+        user_name: "alice",
+        channel_id: "chan-1",
+        post_id: "post-1",
+        context: { ...context, _token: token },
+      },
+    });
+    const res = createRes();
+
+    await handler(req, res);
+
+    expect(res.statusCode).toBe(200);
+    expect(res.body).toBe("{}");
+    expect(requestLog).toEqual([
+      { path: "/posts/post-1", method: undefined },
+      { path: "/posts/post-1", method: "PUT" },
+    ]);
   });
 
-  it("accepts ::1", () => {
-    expect(isLocalhostRequest(fakeReq("::1"))).toBe(true);
-  });
+  it("rejects requests with an invalid interaction token", async () => {
+    const handler = createMattermostInteractionHandler({
+      client: {
+        request: async () => ({ message: "unused" }),
+      } as unknown as MattermostClient,
+      botUserId: "bot",
+      accountId: "acct",
+    });
 
-  it("accepts ::ffff:127.0.0.1", () => {
-    expect(isLocalhostRequest(fakeReq("::ffff:127.0.0.1"))).toBe(true);
-  });
+    const req = createReq({
+      body: {
+        user_id: "user-1",
+        channel_id: "chan-1",
+        post_id: "post-1",
+        context: { action_id: "approve", _token: "deadbeef" },
+      },
+    });
+    const res = createRes();
 
-  it("rejects external addresses", () => {
-    expect(isLocalhostRequest(fakeReq("10.0.0.1"))).toBe(false);
-    expect(isLocalhostRequest(fakeReq("192.168.1.1"))).toBe(false);
-  });
+    await handler(req, res);
 
-  it("rejects when socket has no remote address", () => {
-    expect(isLocalhostRequest(fakeReq(undefined))).toBe(false);
-  });
-
-  it("rejects when socket is missing", () => {
-    expect(isLocalhostRequest({} as IncomingMessage)).toBe(false);
+    expect(res.statusCode).toBe(403);
+    expect(res.body).toContain("Invalid token");
   });
 });

extensions/mattermost/src/mattermost/interactions.ts

@@ -1,5 +1,6 @@
 import { createHmac, timingSafeEqual } from "node:crypto";
 import type { IncomingMessage, ServerResponse } from "node:http";
+import type { OpenClawConfig } from "openclaw/plugin-sdk/mattermost";
 import { getMattermostRuntime } from "../runtime.js";
 import { updateMattermostPost, type MattermostClient } from "./client.js";
 
@@ -43,21 +44,72 @@ export function getInteractionCallbackUrl(accountId: string): string | undefined
   return callbackUrls.get(accountId);
 }
 
+type InteractionCallbackConfig = Pick<OpenClawConfig, "gateway" | "channels"> & {
+  interactions?: {
+    callbackBaseUrl?: string;
+  };
+};
+
+export function resolveInteractionCallbackPath(accountId: string): string {
+  return `/mattermost/interactions/${accountId}`;
+}
+
+function isWildcardBindHost(rawHost: string): boolean {
+  const trimmed = rawHost.trim();
+  if (!trimmed) return false;
+  const host = trimmed.startsWith("[") && trimmed.endsWith("]") ? trimmed.slice(1, -1) : trimmed;
+  return host === "0.0.0.0" || host === "::" || host === "0:0:0:0:0:0:0:0" || host === "::0";
+}
+
+function normalizeCallbackBaseUrl(baseUrl: string): string {
+  return baseUrl.trim().replace(/\/+$/, "");
+}
+
 /**
  * Resolve the interaction callback URL for an account.
- * Prefers the in-memory registered URL (set by the gateway monitor).
- * Falls back to computing it from the gateway port in config (for CLI callers).
+ * Falls back to computing it from interactions.callbackBaseUrl or gateway host config.
+ */
+export function computeInteractionCallbackUrl(
+  accountId: string,
+  cfg?: InteractionCallbackConfig,
+): string {
+  const path = resolveInteractionCallbackPath(accountId);
+  // Prefer merged per-account config when available, but keep the top-level path for
+  // callers/tests that still pass the root Mattermost config shape directly.
+  const callbackBaseUrl =
+    cfg?.interactions?.callbackBaseUrl?.trim() ??
+    cfg?.channels?.mattermost?.interactions?.callbackBaseUrl?.trim();
+  if (callbackBaseUrl) {
+    return `${normalizeCallbackBaseUrl(callbackBaseUrl)}${path}`;
+  }
+  const port = typeof cfg?.gateway?.port === "number" ? cfg.gateway.port : 18789;
+  let host =
+    cfg?.gateway?.customBindHost && !isWildcardBindHost(cfg.gateway.customBindHost)
+      ? cfg.gateway.customBindHost.trim()
+      : "localhost";
+
+  // Bracket IPv6 literals so the URL is valid: http://[::1]:18789/...
+  if (host.includes(":") && !(host.startsWith("[") && host.endsWith("]"))) {
+    host = `[${host}]`;
+  }
+
+  return `http://${host}:${port}${path}`;
+}
+
+/**
+ * Resolve the interaction callback URL for an account.
+ * Prefers the in-memory registered URL (set by the gateway monitor) so callers outside the
+ * monitor lifecycle can reuse the runtime-validated callback destination.
  */
 export function resolveInteractionCallbackUrl(
   accountId: string,
-  cfg?: { gateway?: { port?: number } },
+  cfg?: InteractionCallbackConfig,
 ): string {
   const cached = callbackUrls.get(accountId);
   if (cached) {
     return cached;
   }
-  const port = typeof cfg?.gateway?.port === "number" ? cfg.gateway.port : 18789;
-  return `http://localhost:${port}/mattermost/interactions/${accountId}`;
+  return computeInteractionCallbackUrl(accountId, cfg);
 }
 
 // ── HMAC token management ──────────────────────────────────────────────
@@ -198,18 +250,6 @@ export function buildButtonAttachments(params: {
   ];
 }
 
-// ── Localhost validation ───────────────────────────────────────────────
-
-const LOCALHOST_ADDRESSES = new Set(["127.0.0.1", "::1", "::ffff:127.0.0.1"]);
-
-export function isLocalhostRequest(req: IncomingMessage): boolean {
-  const addr = req.socket?.remoteAddress;
-  if (!addr) {
-    return false;
-  }
-  return LOCALHOST_ADDRESSES.has(addr);
-}
-
 // ── Request body reader ────────────────────────────────────────────────
 
 function readInteractionBody(req: IncomingMessage): Promise<string> {
@@ -251,7 +291,6 @@ export function createMattermostInteractionHandler(params: {
   client: MattermostClient;
   botUserId: string;
   accountId: string;
-  callbackUrl: string;
   resolveSessionKey?: (channelId: string, userId: string) => Promise<string>;
   dispatchButtonClick?: (opts: {
     channelId: string;
@@ -276,17 +315,6 @@ export function createMattermostInteractionHandler(params: {
       return;
     }
 
-    // Verify request is from localhost
-    if (!isLocalhostRequest(req)) {
-      log?.(
-        `mattermost interaction: rejected non-localhost request from ${req.socket?.remoteAddress}`,
-      );
-      res.statusCode = 403;
-      res.setHeader("Content-Type", "application/json");
-      res.end(JSON.stringify({ error: "Forbidden" }));
-      return;
-    }
-
     let payload: MattermostInteractionPayload;
     try {
       const raw = await readInteractionBody(req);

extensions/mattermost/src/mattermost/monitor.ts

@@ -44,7 +44,9 @@ import {
   type MattermostUser,
 } from "./client.js";
 import {
+  computeInteractionCallbackUrl,
   createMattermostInteractionHandler,
+  resolveInteractionCallbackPath,
   setInteractionCallbackUrl,
   setInteractionSecret,
 } from "./interactions.js";
@@ -100,6 +102,10 @@ const RECENT_MATTERMOST_MESSAGE_MAX = 2000;
 const CHANNEL_CACHE_TTL_MS = 5 * 60_000;
 const USER_CACHE_TTL_MS = 10 * 60_000;
 
+function isLoopbackHost(hostname: string): boolean {
+  return hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1";
+}
+
 const recentInboundMessages = createDedupeCache({
   ttlMs: RECENT_MATTERMOST_MESSAGE_TTL_MS,
   maxSize: RECENT_MATTERMOST_MESSAGE_MAX,
@@ -333,9 +339,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
         gatewayHost: cfg.gateway?.customBindHost ?? undefined,
       });
 
-      const isLoopbackHost = (hostname: string) =>
-        hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1";
-
       try {
         const mmHost = new URL(baseUrl).hostname;
         const callbackHost = new URL(slashCallbackUrl).hostname;
@@ -452,10 +455,27 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
 
   // Register HTTP callback endpoint for interactive button clicks.
   // Mattermost POSTs to this URL when a user clicks a button action.
-  const gatewayPort = typeof cfg.gateway?.port === "number" ? cfg.gateway.port : 18789;
-  const interactionPath = `/mattermost/interactions/${account.accountId}`;
-  const callbackUrl = `http://localhost:${gatewayPort}${interactionPath}`;
+  const interactionPath = resolveInteractionCallbackPath(account.accountId);
+  // Recompute from config on each monitor start so reconnects or config reloads can refresh the
+  // cached callback URL for downstream callers such as `message action=send`.
+  const callbackUrl = computeInteractionCallbackUrl(account.accountId, {
+    gateway: cfg.gateway,
+    interactions: account.config.interactions,
+  });
   setInteractionCallbackUrl(account.accountId, callbackUrl);
+
+  try {
+    const mmHost = new URL(baseUrl).hostname;
+    const callbackHost = new URL(callbackUrl).hostname;
+    if (isLoopbackHost(callbackHost) && !isLoopbackHost(mmHost)) {
+      runtime.error?.(
+        `mattermost: interactions callbackUrl resolved to ${callbackUrl} (loopback) while baseUrl is ${baseUrl}. This MAY be unreachable depending on your deployment. If button clicks don't work, set channels.mattermost.interactions.callbackBaseUrl to a URL reachable from the Mattermost server (e.g. your public reverse proxy URL).`,
+      );
+    }
+  } catch {
+    // URL parse failed; ignore and continue (we will fail naturally if callbacks cannot be delivered).
+  }
+
   const unregisterInteractions = registerPluginHttpRoute({
     path: interactionPath,
     fallbackPath: "/mattermost/interactions/default",
@@ -464,7 +484,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
       client,
       botUserId,
       accountId: account.accountId,
-      callbackUrl,
       resolveSessionKey: async (channelId: string, userId: string) => {
         const channelInfo = await resolveChannelInfo(channelId);
         const kind = mapMattermostChannelTypeToChatType(channelInfo?.type);