diff --git a/CHANGELOG.md b/CHANGELOG.md index 05cef55abea..29c8dfad271 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,6 +120,7 @@ Docs: https://docs.openclaw.ai - LINE/context and routing synthesis: fix group/room peer routing and command-authorization context propagation, and keep processing later events in mixed-success webhook batches. (from #21955, #24475, #27035, #28286) Thanks @lailoo, @mcaxtr, @jervyclaw, @Glucksberg, and @Takhoffman. - LINE/status/config/webhook synthesis: fix status false positives from snapshot/config state and accept LINE webhook HEAD probes for compatibility. (from #10487, #25726, #27537, #27908, #31387) Thanks @BlueBirdBack, @stakeswky, @loiie45e, @puritysb, and @mcaxtr. - LINE cleanup/test follow-ups: fold cleanup/test learnings into the synthesis review path while keeping runtime changes focused on regression fixes. (from #17630, #17289) Thanks @Clawborn and @davidahmann. +- Mattermost/interactive buttons hardening: scope interaction HMAC secrets per account, route DM button-click sessions by sender identity, and restore Mattermost send test mocks for channel-name resolution coverage. (#19957) thanks @tonydehnke. ## 2026.3.2 diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index 401ddabf38b..5897c11277a 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -164,7 +164,7 @@ const mattermostMessageActions: ChannelMessageActionAdapter = { let props: Record | undefined; if (params.buttons && Array.isArray(params.buttons)) { const account = resolveMattermostAccount({ cfg, accountId: resolvedAccountId }); - if (account.botToken) setInteractionSecret(account.botToken); + if (account.botToken) setInteractionSecret(account.accountId, account.botToken); const callbackUrl = resolveInteractionCallbackUrl(account.accountId, cfg); // Flatten 2D array (rows of buttons) to 1D — core schema sends Array> @@ -191,6 +191,7 @@ const mattermostMessageActions: ChannelMessageActionAdapter = { props = { attachments: buildButtonAttachments({ callbackUrl, + accountId: account.accountId, buttons, text: attachmentText, }), diff --git a/extensions/mattermost/src/mattermost/interactions.test.ts b/extensions/mattermost/src/mattermost/interactions.test.ts index c3aefca32ae..0e24ae4a4ee 100644 --- a/extensions/mattermost/src/mattermost/interactions.test.ts +++ b/extensions/mattermost/src/mattermost/interactions.test.ts @@ -103,6 +103,16 @@ describe("generateInteractionToken / verifyInteractionToken", () => { const reorderedContext = { action: "do", action_id: "bm_do", tweet_id: "999" }; expect(verifyInteractionToken(reorderedContext, token)).toBe(true); }); + + it("scopes tokens per account when account secrets differ", () => { + setInteractionSecret("acct-a", "bot-token-a"); + setInteractionSecret("acct-b", "bot-token-b"); + const context = { action_id: "do_now", item_id: "123" }; + const tokenA = generateInteractionToken(context, "acct-a"); + + expect(verifyInteractionToken(context, tokenA, "acct-a")).toBe(true); + expect(verifyInteractionToken(context, tokenA, "acct-b")).toBe(false); + }); }); // ── Callback URL registry ──────────────────────────────────────────── diff --git a/extensions/mattermost/src/mattermost/interactions.ts b/extensions/mattermost/src/mattermost/interactions.ts index 5a52e630a59..be305db4ba3 100644 --- a/extensions/mattermost/src/mattermost/interactions.ts +++ b/extensions/mattermost/src/mattermost/interactions.ts @@ -63,32 +63,58 @@ export function resolveInteractionCallbackUrl( // ── HMAC token management ────────────────────────────────────────────── // Secret is derived from the bot token so it's stable across CLI and gateway processes. -let interactionSecret: string | undefined; +const interactionSecrets = new Map(); +let defaultInteractionSecret: string | undefined; -export function setInteractionSecret(botToken: string): void { - interactionSecret = createHmac("sha256", "openclaw-mattermost-interactions") - .update(botToken) - .digest("hex"); +function deriveInteractionSecret(botToken: string): string { + return createHmac("sha256", "openclaw-mattermost-interactions").update(botToken).digest("hex"); } -export function getInteractionSecret(): string { - if (!interactionSecret) { - throw new Error( - "Interaction secret not initialized — call setInteractionSecret(botToken) first", - ); +export function setInteractionSecret(accountIdOrBotToken: string, botToken?: string): void { + if (typeof botToken === "string") { + interactionSecrets.set(accountIdOrBotToken, deriveInteractionSecret(botToken)); + return; } - return interactionSecret; + // Backward-compatible fallback for call sites/tests that only pass botToken. + defaultInteractionSecret = deriveInteractionSecret(accountIdOrBotToken); } -export function generateInteractionToken(context: Record): string { - const secret = getInteractionSecret(); +export function getInteractionSecret(accountId?: string): string { + const scoped = accountId ? interactionSecrets.get(accountId) : undefined; + if (scoped) { + return scoped; + } + if (defaultInteractionSecret) { + return defaultInteractionSecret; + } + // Fallback for single-account runtimes that only registered scoped secrets. + if (interactionSecrets.size === 1) { + const first = interactionSecrets.values().next().value; + if (typeof first === "string") { + return first; + } + } + throw new Error( + "Interaction secret not initialized — call setInteractionSecret(accountId, botToken) first", + ); +} + +export function generateInteractionToken( + context: Record, + accountId?: string, +): string { + const secret = getInteractionSecret(accountId); // Sort keys for stable serialization — Mattermost may reorder context keys const payload = JSON.stringify(context, Object.keys(context).sort()); return createHmac("sha256", secret).update(payload).digest("hex"); } -export function verifyInteractionToken(context: Record, token: string): boolean { - const expected = generateInteractionToken(context); +export function verifyInteractionToken( + context: Record, + token: string, + accountId?: string, +): boolean { + const expected = generateInteractionToken(context, accountId); if (expected.length !== token.length) { return false; } @@ -133,6 +159,7 @@ function sanitizeActionId(id: string): string { export function buildButtonAttachments(params: { callbackUrl: string; + accountId?: string; buttons: Array<{ id: string; name: string; @@ -147,7 +174,7 @@ export function buildButtonAttachments(params: { action_id: safeId, ...btn.context, }; - const token = generateInteractionToken(context); + const token = generateInteractionToken(context, params.accountId); return { id: safeId, type: "button" as const, @@ -225,7 +252,7 @@ export function createMattermostInteractionHandler(params: { botUserId: string; accountId: string; callbackUrl: string; - resolveSessionKey?: (channelId: string) => Promise; + resolveSessionKey?: (channelId: string, userId: string) => Promise; dispatchButtonClick?: (opts: { channelId: string; userId: string; @@ -236,7 +263,7 @@ export function createMattermostInteractionHandler(params: { }) => Promise; log?: (message: string) => void; }): (req: IncomingMessage, res: ServerResponse) => Promise { - const { client, botUserId, accountId, log } = params; + const { client, accountId, log } = params; const core = getMattermostRuntime(); return async (req: IncomingMessage, res: ServerResponse) => { @@ -292,7 +319,7 @@ export function createMattermostInteractionHandler(params: { // Strip _token before verification (it wasn't in the original context) const { _token, ...contextWithoutToken } = context; - if (!verifyInteractionToken(contextWithoutToken, token)) { + if (!verifyInteractionToken(contextWithoutToken, token, accountId)) { log?.("mattermost interaction: invalid _token"); res.statusCode = 403; res.setHeader("Content-Type", "application/json"); @@ -323,7 +350,7 @@ export function createMattermostInteractionHandler(params: { `in channel ${payload.channel_id}`; const sessionKey = params.resolveSessionKey - ? await params.resolveSessionKey(payload.channel_id) + ? await params.resolveSessionKey(payload.channel_id, payload.user_id) : `agent:main:mattermost:${accountId}:${payload.channel_id}`; core.system.enqueueSystemEvent(eventLabel, { diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 1a3a50ba035..13864a33f44 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -448,7 +448,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} // ─── Interactive buttons registration ────────────────────────────────────── // Derive a stable HMAC secret from the bot token so CLI and gateway share it. - setInteractionSecret(botToken); + setInteractionSecret(account.accountId, botToken); // Register HTTP callback endpoint for interactive button clicks. // Mattermost POSTs to this URL when a user clicks a button action. @@ -465,7 +465,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} botUserId, accountId: account.accountId, callbackUrl, - resolveSessionKey: async (channelId: string) => { + resolveSessionKey: async (channelId: string, userId: string) => { const channelInfo = await resolveChannelInfo(channelId); const kind = mapMattermostChannelTypeToChatType(channelInfo?.type); const teamId = channelInfo?.team_id ?? undefined; @@ -476,7 +476,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} teamId, peer: { kind, - id: kind === "direct" ? botUserId : channelId, + id: kind === "direct" ? userId : channelId, }, }); return route.sessionKey; @@ -495,7 +495,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} teamId, peer: { kind, - id: kind === "direct" ? botUserId : opts.channelId, + id: kind === "direct" ? opts.userId : opts.channelId, }, }); const to = kind === "direct" ? `user:${opts.userId}` : `channel:${opts.channelId}`; diff --git a/extensions/mattermost/src/mattermost/send.test.ts b/extensions/mattermost/src/mattermost/send.test.ts index ce2044f749d..364a4c91744 100644 --- a/extensions/mattermost/src/mattermost/send.test.ts +++ b/extensions/mattermost/src/mattermost/send.test.ts @@ -12,7 +12,9 @@ const mockState = vi.hoisted(() => ({ createMattermostClient: vi.fn(), createMattermostDirectChannel: vi.fn(), createMattermostPost: vi.fn(), + fetchMattermostChannelByName: vi.fn(), fetchMattermostMe: vi.fn(), + fetchMattermostUserTeams: vi.fn(), fetchMattermostUserByUsername: vi.fn(), normalizeMattermostBaseUrl: vi.fn((input: string | undefined) => input?.trim() ?? ""), uploadMattermostFile: vi.fn(), @@ -30,7 +32,9 @@ vi.mock("./client.js", () => ({ createMattermostClient: mockState.createMattermostClient, createMattermostDirectChannel: mockState.createMattermostDirectChannel, createMattermostPost: mockState.createMattermostPost, + fetchMattermostChannelByName: mockState.fetchMattermostChannelByName, fetchMattermostMe: mockState.fetchMattermostMe, + fetchMattermostUserTeams: mockState.fetchMattermostUserTeams, fetchMattermostUserByUsername: mockState.fetchMattermostUserByUsername, normalizeMattermostBaseUrl: mockState.normalizeMattermostBaseUrl, uploadMattermostFile: mockState.uploadMattermostFile, @@ -71,11 +75,16 @@ describe("sendMessageMattermost", () => { mockState.createMattermostClient.mockReset(); mockState.createMattermostDirectChannel.mockReset(); mockState.createMattermostPost.mockReset(); + mockState.fetchMattermostChannelByName.mockReset(); mockState.fetchMattermostMe.mockReset(); + mockState.fetchMattermostUserTeams.mockReset(); mockState.fetchMattermostUserByUsername.mockReset(); mockState.uploadMattermostFile.mockReset(); mockState.createMattermostClient.mockReturnValue({}); mockState.createMattermostPost.mockResolvedValue({ id: "post-1" }); + mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-user" }); + mockState.fetchMattermostUserTeams.mockResolvedValue([{ id: "team-1" }]); + mockState.fetchMattermostChannelByName.mockResolvedValue({ id: "town-square" }); mockState.uploadMattermostFile.mockResolvedValue({ id: "file-1" }); });