Mattermost: harden interaction callback binding (#38057)

extensions/mattermost/src/channel.ts

@@ -34,11 +34,13 @@ import {
 import { monitorMattermostProvider } from "./mattermost/monitor.js";
 import { probeMattermost } from "./mattermost/probe.js";
 import { addMattermostReaction, removeMattermostReaction } from "./mattermost/reactions.js";
-import { sendMessageMattermost } from "./mattermost/send.js";
+import { resolveMattermostSendChannelId, sendMessageMattermost } from "./mattermost/send.js";
 import { looksLikeMattermostTargetId, normalizeMattermostMessagingTarget } from "./normalize.js";
 import { mattermostOnboardingAdapter } from "./onboarding.js";
 import { getMattermostRuntime } from "./runtime.js";
 
+const SIGNED_CHANNEL_ID_CONTEXT_KEY = "__openclaw_channel_id";
+
 const mattermostMessageActions: ChannelMessageActionAdapter = {
   listActions: ({ cfg }) => {
     const enabledAccounts = listMattermostAccountIds(cfg)
@@ -165,6 +167,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 channelId = await resolveMattermostSendChannelId(to, {
+        cfg,
+        accountId: account.accountId,
+      });
       const callbackUrl = resolveInteractionCallbackUrl(account.accountId, {
         gateway: cfg.gateway,
         interactions: account.config.interactions,
@@ -184,8 +190,11 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
           style: (btn.style as "default" | "primary" | "danger") ?? "default",
           context:
             typeof btn.context === "object" && btn.context !== null
-              ? (btn.context as Record<string, unknown>)
-              : undefined,
+              ? {
+                  ...(btn.context as Record<string, unknown>),
+                  [SIGNED_CHANNEL_ID_CONTEXT_KEY]: channelId,
+                }
+              : { [SIGNED_CHANNEL_ID_CONTEXT_KEY]: channelId },
         }))
         .filter((btn) => btn.id && btn.name);
 

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

@@ -448,7 +448,7 @@ describe("createMattermostInteractionHandler", () => {
   }
 
   it("accepts non-localhost requests when the interaction token is valid", async () => {
-    const context = { action_id: "approve" };
+    const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
     const token = generateInteractionToken(context, "acct");
     const requestLog: Array<{ path: string; method?: string }> = [];
     const handler = createMattermostInteractionHandler({
@@ -459,6 +459,7 @@ describe("createMattermostInteractionHandler", () => {
             return { id: "post-1" };
           }
           return {
+            channel_id: "chan-1",
             message: "Choose",
             props: {
               attachments: [{ actions: [{ id: "approve", name: "Approve" }] }],
@@ -516,4 +517,97 @@ describe("createMattermostInteractionHandler", () => {
     expect(res.statusCode).toBe(403);
     expect(res.body).toContain("Invalid token");
   });
+
+  it("rejects requests when the signed channel does not match the callback payload", async () => {
+    const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
+    const token = generateInteractionToken(context, "acct");
+    const handler = createMattermostInteractionHandler({
+      client: {
+        request: async () => ({ message: "unused" }),
+      } as unknown as MattermostClient,
+      botUserId: "bot",
+      accountId: "acct",
+    });
+
+    const req = createReq({
+      body: {
+        user_id: "user-1",
+        channel_id: "chan-2",
+        post_id: "post-1",
+        context: { ...context, _token: token },
+      },
+    });
+    const res = createRes();
+
+    await handler(req, res);
+
+    expect(res.statusCode).toBe(403);
+    expect(res.body).toContain("Channel mismatch");
+  });
+
+  it("rejects requests when the fetched post does not belong to the callback channel", async () => {
+    const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
+    const token = generateInteractionToken(context, "acct");
+    const handler = createMattermostInteractionHandler({
+      client: {
+        request: async () => ({
+          channel_id: "chan-9",
+          message: "Choose",
+          props: {
+            attachments: [{ actions: [{ id: "approve", name: "Approve" }] }],
+          },
+        }),
+      } as unknown as MattermostClient,
+      botUserId: "bot",
+      accountId: "acct",
+    });
+
+    const req = createReq({
+      body: {
+        user_id: "user-1",
+        channel_id: "chan-1",
+        post_id: "post-1",
+        context: { ...context, _token: token },
+      },
+    });
+    const res = createRes();
+
+    await handler(req, res);
+
+    expect(res.statusCode).toBe(403);
+    expect(res.body).toContain("Post/channel mismatch");
+  });
+
+  it("rejects requests when the action is not present on the fetched post", async () => {
+    const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
+    const token = generateInteractionToken(context, "acct");
+    const handler = createMattermostInteractionHandler({
+      client: {
+        request: async () => ({
+          channel_id: "chan-1",
+          message: "Choose",
+          props: {
+            attachments: [{ actions: [{ id: "reject", name: "Reject" }] }],
+          },
+        }),
+      } as unknown as MattermostClient,
+      botUserId: "bot",
+      accountId: "acct",
+    });
+
+    const req = createReq({
+      body: {
+        user_id: "user-1",
+        channel_id: "chan-1",
+        post_id: "post-1",
+        context: { ...context, _token: token },
+      },
+    });
+    const res = createRes();
+
+    await handler(req, res);
+
+    expect(res.statusCode).toBe(403);
+    expect(res.body).toContain("Unknown action");
+  });
 });

extensions/mattermost/src/mattermost/interactions.ts

@@ -6,6 +6,7 @@ import { updateMattermostPost, type MattermostClient } from "./client.js";
 
 const INTERACTION_MAX_BODY_BYTES = 64 * 1024;
 const INTERACTION_BODY_TIMEOUT_MS = 10_000;
+const SIGNED_CHANNEL_ID_CONTEXT_KEY = "__openclaw_channel_id";
 
 /**
  * Mattermost interactive message callback payload.
@@ -363,6 +364,69 @@ export function createMattermostInteractionHandler(params: {
       return;
     }
 
+    const signedChannelId =
+      typeof contextWithoutToken[SIGNED_CHANNEL_ID_CONTEXT_KEY] === "string"
+        ? contextWithoutToken[SIGNED_CHANNEL_ID_CONTEXT_KEY].trim()
+        : "";
+    if (signedChannelId && signedChannelId !== payload.channel_id) {
+      log?.(
+        `mattermost interaction: signed channel mismatch payload=${payload.channel_id} signed=${signedChannelId}`,
+      );
+      res.statusCode = 403;
+      res.setHeader("Content-Type", "application/json");
+      res.end(JSON.stringify({ error: "Channel mismatch" }));
+      return;
+    }
+
+    const userName = payload.user_name ?? payload.user_id;
+    let originalMessage = "";
+    let clickedButtonName = actionId;
+    try {
+      const originalPost = await client.request<{
+        channel_id?: string | null;
+        message?: string;
+        props?: Record<string, unknown>;
+      }>(`/posts/${payload.post_id}`);
+      const postChannelId = originalPost.channel_id?.trim();
+      if (!postChannelId || postChannelId !== payload.channel_id) {
+        log?.(
+          `mattermost interaction: post channel mismatch payload=${payload.channel_id} post=${postChannelId ?? "<missing>"}`,
+        );
+        res.statusCode = 403;
+        res.setHeader("Content-Type", "application/json");
+        res.end(JSON.stringify({ error: "Post/channel mismatch" }));
+        return;
+      }
+      originalMessage = originalPost.message ?? "";
+
+      // Ensure the callback can only target an action that exists on the original post.
+      const postAttachments = Array.isArray(originalPost?.props?.attachments)
+        ? (originalPost.props.attachments as Array<{
+            actions?: Array<{ id?: string; name?: string }>;
+          }>)
+        : [];
+      for (const att of postAttachments) {
+        const match = att.actions?.find((a) => a.id === actionId);
+        if (match?.name) {
+          clickedButtonName = match.name;
+          break;
+        }
+      }
+      if (clickedButtonName === actionId) {
+        log?.(`mattermost interaction: action ${actionId} not found in post ${payload.post_id}`);
+        res.statusCode = 403;
+        res.setHeader("Content-Type", "application/json");
+        res.end(JSON.stringify({ error: "Unknown action" }));
+        return;
+      }
+    } catch (err) {
+      log?.(`mattermost interaction: failed to validate post ${payload.post_id}: ${String(err)}`);
+      res.statusCode = 500;
+      res.setHeader("Content-Type", "application/json");
+      res.end(JSON.stringify({ error: "Failed to validate interaction" }));
+      return;
+    }
+
     log?.(
       `mattermost interaction: action=${actionId} user=${payload.user_name ?? payload.user_id} ` +
         `post=${payload.post_id} channel=${payload.channel_id}`,
@@ -389,34 +453,6 @@ export function createMattermostInteractionHandler(params: {
       log?.(`mattermost interaction: system event dispatch failed: ${String(err)}`);
     }
 
-    // Fetch the original post to preserve its message and find the clicked button name.
-    const userName = payload.user_name ?? payload.user_id;
-    let originalMessage = "";
-    let clickedButtonName = actionId; // fallback to action ID if we can't find the name
-    try {
-      const originalPost = await client.request<{
-        message?: string;
-        props?: Record<string, unknown>;
-      }>(`/posts/${payload.post_id}`);
-      originalMessage = originalPost?.message ?? "";
-
-      // Find the clicked button's display name from the original attachments
-      const postAttachments = Array.isArray(originalPost?.props?.attachments)
-        ? (originalPost.props.attachments as Array<{
-            actions?: Array<{ id?: string; name?: string }>;
-          }>)
-        : [];
-      for (const att of postAttachments) {
-        const match = att.actions?.find((a) => a.id === actionId);
-        if (match?.name) {
-          clickedButtonName = match.name;
-          break;
-        }
-      }
-    } catch (err) {
-      log?.(`mattermost interaction: failed to fetch post ${payload.post_id}: ${String(err)}`);
-    }
-
     // Update the post via API to replace buttons with a completion indicator.
     try {
       await updateMattermostPost(client, payload.post_id, {

extensions/mattermost/src/mattermost/monitor.ts

@@ -544,7 +544,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
           Surface: "mattermost" as const,
           MessageSid: `interaction:${opts.postId}:${opts.actionId}`,
           WasMentioned: true,
-          CommandAuthorized: true,
+          CommandAuthorized: false,
           OriginatingChannel: "mattermost" as const,
           OriginatingTo: to,
         });

extensions/mattermost/src/mattermost/send.ts

@@ -205,13 +205,19 @@ async function resolveTargetChannelId(params: {
   return channel.id;
 }
 
-export async function sendMessageMattermost(
+type MattermostSendContext = {
+  cfg: OpenClawConfig;
+  accountId: string;
+  token: string;
+  baseUrl: string;
+  channelId: string;
+};
+
+async function resolveMattermostSendContext(
   to: string,
-  text: string,
   opts: MattermostSendOpts = {},
-): Promise<MattermostSendResult> {
+): Promise<MattermostSendContext> {
   const core = getCore();
-  const logger = core.logging.getChildLogger({ module: "mattermost" });
   const cfg = opts.cfg ?? core.config.loadConfig();
   const account = resolveMattermostAccount({
     cfg,
@@ -237,6 +243,34 @@ export async function sendMessageMattermost(
     token,
   });
 
+  return {
+    cfg,
+    accountId: account.accountId,
+    token,
+    baseUrl,
+    channelId,
+  };
+}
+
+export async function resolveMattermostSendChannelId(
+  to: string,
+  opts: MattermostSendOpts = {},
+): Promise<string> {
+  return (await resolveMattermostSendContext(to, opts)).channelId;
+}
+
+export async function sendMessageMattermost(
+  to: string,
+  text: string,
+  opts: MattermostSendOpts = {},
+): Promise<MattermostSendResult> {
+  const core = getCore();
+  const logger = core.logging.getChildLogger({ module: "mattermost" });
+  const { cfg, accountId, token, baseUrl, channelId } = await resolveMattermostSendContext(
+    to,
+    opts,
+  );
+
   const client = createMattermostClient({ baseUrl, botToken: token });
   let message = text?.trim() ?? "";
   let fileIds: string[] | undefined;
@@ -269,7 +303,7 @@ export async function sendMessageMattermost(
     const tableMode = core.channel.text.resolveMarkdownTableMode({
       cfg,
       channel: "mattermost",
-      accountId: account.accountId,
+      accountId,
     });
     message = core.channel.text.convertMarkdownTables(message, tableMode);
   }
@@ -291,7 +325,7 @@ export async function sendMessageMattermost(
 
   core.channel.activity.record({
     channel: "mattermost",
-    accountId: account.accountId,
+    accountId,
     direction: "outbound",
   });