From dadef27d7abaf107366a1c81b98ed7f2f1078d48 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 14 Jan 2026 15:53:45 +0000 Subject: [PATCH] fix(slack): drop mismatched Socket Mode events (#889) Filter Slack Socket Mode events by api_app_id/team_id. Refs: #828 Contributor: @roshanasingh4 Co-authored-by: Roshan Singh --- CHANGELOG.md | 1 + ...ends-tool-summaries-responseprefix.test.ts | 40 ++++++++++++ src/slack/monitor/context.ts | 24 +++++++ src/slack/monitor/events/channels.ts | 57 +++++++++------- src/slack/monitor/events/members.ts | 6 +- src/slack/monitor/events/messages.ts | 9 ++- src/slack/monitor/events/pins.ts | 8 ++- src/slack/monitor/events/reactions.ts | 65 ++++++------------- src/slack/monitor/provider.ts | 17 +++++ 9 files changed, 152 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1028f42b..df0d3a941 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - macOS: fix cron preview/testing payload to use `channel` key. (#867) — thanks @wes-davis. - Telegram: honor `channels.telegram.timeoutSeconds` for grammY API requests. (#863) — thanks @Snaver. - Telegram: split long captions into media + follow-up text messages. (#907) - thanks @jalehman. +- Slack: drop Socket Mode events with mismatched `api_app_id`/`team_id`. (#889) — thanks @roshanasingh4. ## 2026.1.13 diff --git a/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts b/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts index 11d4e8585..01e36052b 100644 --- a/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts +++ b/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts @@ -158,6 +158,46 @@ describe("monitorSlackProvider tool results", () => { expect(sendMock.mock.calls[1][1]).toBe("PFX final reply"); }); + it("drops events with mismatched api_app_id", async () => { + const client = getSlackClient(); + if (!client) throw new Error("Slack client not registered"); + (client.auth as { test: ReturnType }).test.mockResolvedValue({ + user_id: "bot-user", + team_id: "T1", + api_app_id: "A1", + }); + + const controller = new AbortController(); + const run = monitorSlackProvider({ + botToken: "bot-token", + appToken: "xapp-1-A1-abc", + abortSignal: controller.signal, + }); + + await waitForEvent("message"); + const handler = getSlackHandlers()?.get("message"); + if (!handler) throw new Error("Slack message handler not registered"); + + await handler({ + body: { api_app_id: "A2", team_id: "T1" }, + event: { + type: "message", + user: "U1", + text: "hello", + ts: "123", + channel: "C1", + channel_type: "im", + }, + }); + + await flush(); + controller.abort(); + await run; + + expect(sendMock).not.toHaveBeenCalled(); + expect(replyMock).not.toHaveBeenCalled(); + }); + it("does not derive responsePrefix from routed agent identity when unset", async () => { config = { agents: { diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index 21a5c6ede..5f96f68af 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -22,6 +22,7 @@ export type SlackMonitorContext = { botUserId: string; teamId: string; + apiAppId: string; historyLimit: number; channelHistories: Map; @@ -58,6 +59,7 @@ export type SlackMonitorContext = { logger: ReturnType; markMessageSeen: (channelId: string | undefined, ts?: string) => boolean; + shouldDropMismatchedSlackEvent: (body: unknown) => boolean; resolveSlackSystemEventSessionKey: (params: { channelId?: string | null; channelType?: string | null; @@ -90,6 +92,7 @@ export function createSlackMonitorContext(params: { botUserId: string; teamId: string; + apiAppId: string; historyLimit: number; sessionScope: SessionScope; @@ -290,6 +293,25 @@ export function createSlackMonitorContext(params: { return true; }; + const shouldDropMismatchedSlackEvent = (body: unknown) => { + if (!body || typeof body !== "object") return false; + const raw = body as { api_app_id?: unknown; team_id?: unknown }; + const incomingApiAppId = typeof raw.api_app_id === "string" ? raw.api_app_id : ""; + const incomingTeamId = typeof raw.team_id === "string" ? raw.team_id : ""; + + if (params.apiAppId && incomingApiAppId && incomingApiAppId !== params.apiAppId) { + logVerbose( + `slack: drop event with api_app_id=${incomingApiAppId} (expected ${params.apiAppId})`, + ); + return true; + } + if (params.teamId && incomingTeamId && incomingTeamId !== params.teamId) { + logVerbose(`slack: drop event with team_id=${incomingTeamId} (expected ${params.teamId})`); + return true; + } + return false; + }; + return { cfg: params.cfg, accountId: params.accountId, @@ -298,6 +320,7 @@ export function createSlackMonitorContext(params: { runtime: params.runtime, botUserId: params.botUserId, teamId: params.teamId, + apiAppId: params.apiAppId, historyLimit: params.historyLimit, channelHistories, sessionScope: params.sessionScope, @@ -320,6 +343,7 @@ export function createSlackMonitorContext(params: { removeAckAfterReply: params.removeAckAfterReply, logger, markMessageSeen, + shouldDropMismatchedSlackEvent, resolveSlackSystemEventSessionKey, isChannelAllowed, resolveChannelName, diff --git a/src/slack/monitor/events/channels.ts b/src/slack/monitor/events/channels.ts index 381a0d6bf..ec2e3b270 100644 --- a/src/slack/monitor/events/channels.ts +++ b/src/slack/monitor/events/channels.ts @@ -12,8 +12,10 @@ export function registerSlackChannelEvents(params: { ctx: SlackMonitorContext }) ctx.app.event( "channel_created", - async ({ event }: SlackEventMiddlewareArgs<"channel_created">) => { + async ({ event, body }: SlackEventMiddlewareArgs<"channel_created">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + const payload = event as SlackChannelCreatedEvent; const channelId = payload.channel?.id; const channelName = payload.channel?.name; @@ -41,31 +43,36 @@ export function registerSlackChannelEvents(params: { ctx: SlackMonitorContext }) }, ); - ctx.app.event("channel_rename", async ({ event }: SlackEventMiddlewareArgs<"channel_rename">) => { - try { - const payload = event as SlackChannelRenamedEvent; - const channelId = payload.channel?.id; - const channelName = payload.channel?.name_normalized ?? payload.channel?.name; - if ( - !ctx.isChannelAllowed({ + ctx.app.event( + "channel_rename", + async ({ event, body }: SlackEventMiddlewareArgs<"channel_rename">) => { + try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + + const payload = event as SlackChannelRenamedEvent; + const channelId = payload.channel?.id; + const channelName = payload.channel?.name_normalized ?? payload.channel?.name; + if ( + !ctx.isChannelAllowed({ + channelId, + channelName, + channelType: "channel", + }) + ) { + return; + } + const label = resolveSlackChannelLabel({ channelId, channelName }); + const sessionKey = ctx.resolveSlackSystemEventSessionKey({ channelId, - channelName, channelType: "channel", - }) - ) { - return; + }); + enqueueSystemEvent(`Slack channel renamed: ${label}.`, { + sessionKey, + contextKey: `slack:channel:renamed:${channelId ?? channelName ?? "unknown"}`, + }); + } catch (err) { + ctx.runtime.error?.(danger(`slack channel rename handler failed: ${String(err)}`)); } - const label = resolveSlackChannelLabel({ channelId, channelName }); - const sessionKey = ctx.resolveSlackSystemEventSessionKey({ - channelId, - channelType: "channel", - }); - enqueueSystemEvent(`Slack channel renamed: ${label}.`, { - sessionKey, - contextKey: `slack:channel:renamed:${channelId ?? channelName ?? "unknown"}`, - }); - } catch (err) { - ctx.runtime.error?.(danger(`slack channel rename handler failed: ${String(err)}`)); - } - }); + }, + ); } diff --git a/src/slack/monitor/events/members.ts b/src/slack/monitor/events/members.ts index cc3beccd8..c26e815ee 100644 --- a/src/slack/monitor/events/members.ts +++ b/src/slack/monitor/events/members.ts @@ -12,8 +12,9 @@ export function registerSlackMemberEvents(params: { ctx: SlackMonitorContext }) ctx.app.event( "member_joined_channel", - async ({ event }: SlackEventMiddlewareArgs<"member_joined_channel">) => { + async ({ event, body }: SlackEventMiddlewareArgs<"member_joined_channel">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; const payload = event as SlackMemberChannelEvent; const channelId = payload.channel; const channelInfo = channelId ? await ctx.resolveChannelName(channelId) : {}; @@ -49,8 +50,9 @@ export function registerSlackMemberEvents(params: { ctx: SlackMonitorContext }) ctx.app.event( "member_left_channel", - async ({ event }: SlackEventMiddlewareArgs<"member_left_channel">) => { + async ({ event, body }: SlackEventMiddlewareArgs<"member_left_channel">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; const payload = event as SlackMemberChannelEvent; const channelId = payload.channel; const channelInfo = channelId ? await ctx.resolveChannelName(channelId) : {}; diff --git a/src/slack/monitor/events/messages.ts b/src/slack/monitor/events/messages.ts index bcd010319..514f0e0d2 100644 --- a/src/slack/monitor/events/messages.ts +++ b/src/slack/monitor/events/messages.ts @@ -19,8 +19,10 @@ export function registerSlackMessageEvents(params: { }) { const { ctx, handleSlackMessage } = params; - ctx.app.event("message", async ({ event }: SlackEventMiddlewareArgs<"message">) => { + ctx.app.event("message", async ({ event, body }: SlackEventMiddlewareArgs<"message">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + const message = event as SlackMessageEvent; if (message.subtype === "message_changed") { const changed = event as SlackMessageChangedEvent; @@ -108,14 +110,17 @@ export function registerSlackMessageEvents(params: { }); return; } + await handleSlackMessage(message, { source: "message" }); } catch (err) { ctx.runtime.error?.(danger(`slack handler failed: ${String(err)}`)); } }); - ctx.app.event("app_mention", async ({ event }: SlackEventMiddlewareArgs<"app_mention">) => { + ctx.app.event("app_mention", async ({ event, body }: SlackEventMiddlewareArgs<"app_mention">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + const mention = event as SlackAppMentionEvent; await handleSlackMessage(mention as unknown as SlackMessageEvent, { source: "app_mention", diff --git a/src/slack/monitor/events/pins.ts b/src/slack/monitor/events/pins.ts index eb6b4aa64..886db4383 100644 --- a/src/slack/monitor/events/pins.ts +++ b/src/slack/monitor/events/pins.ts @@ -10,8 +10,10 @@ import type { SlackPinEvent } from "../types.js"; export function registerSlackPinEvents(params: { ctx: SlackMonitorContext }) { const { ctx } = params; - ctx.app.event("pin_added", async ({ event }: SlackEventMiddlewareArgs<"pin_added">) => { + ctx.app.event("pin_added", async ({ event, body }: SlackEventMiddlewareArgs<"pin_added">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + const payload = event as SlackPinEvent; const channelId = payload.channel_id; const channelInfo = channelId ? await ctx.resolveChannelName(channelId) : {}; @@ -45,8 +47,10 @@ export function registerSlackPinEvents(params: { ctx: SlackMonitorContext }) { } }); - ctx.app.event("pin_removed", async ({ event }: SlackEventMiddlewareArgs<"pin_removed">) => { + ctx.app.event("pin_removed", async ({ event, body }: SlackEventMiddlewareArgs<"pin_removed">) => { try { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + const payload = event as SlackPinEvent; const channelId = payload.channel_id; const channelInfo = channelId ? await ctx.resolveChannelName(channelId) : {}; diff --git a/src/slack/monitor/events/reactions.ts b/src/slack/monitor/events/reactions.ts index 5aab4be81..ec859051f 100644 --- a/src/slack/monitor/events/reactions.ts +++ b/src/slack/monitor/events/reactions.ts @@ -3,65 +3,37 @@ import type { SlackEventMiddlewareArgs } from "@slack/bolt"; import { danger } from "../../../globals.js"; import { enqueueSystemEvent } from "../../../infra/system-events.js"; -import { normalizeSlackSlug } from "../allow-list.js"; -import { - resolveSlackChannelConfig, - shouldEmitSlackReactionNotification, -} from "../channel-config.js"; +import { resolveSlackChannelLabel } from "../channel-config.js"; import type { SlackMonitorContext } from "../context.js"; -import type { SlackReactionEvent } from "../types.js"; +import type { SlackMessageEvent, SlackReactionEvent } from "../types.js"; export function registerSlackReactionEvents(params: { ctx: SlackMonitorContext }) { const { ctx } = params; - const handleReactionEvent = async (event: SlackReactionEvent, action: "added" | "removed") => { + const handleReactionEvent = async (event: SlackReactionEvent, action: string) => { try { const item = event.item; - if (!event.user) return; - if (!item?.channel || !item?.ts) return; - if (item.type && item.type !== "message") return; - if (ctx.botUserId && event.user === ctx.botUserId) return; - - const channelInfo = await ctx.resolveChannelName(item.channel); - const channelType = channelInfo?.type; - const channelName = channelInfo?.name; + if (!item || item.type !== "message") return; + const channelInfo = item.channel ? await ctx.resolveChannelName(item.channel) : {}; + const channelType = channelInfo?.type as SlackMessageEvent["channel_type"]; if ( !ctx.isChannelAllowed({ channelId: item.channel, - channelName, + channelName: channelInfo?.name, channelType, }) ) { return; } - const isRoom = channelType === "channel" || channelType === "group"; - if (isRoom) { - const channelConfig = resolveSlackChannelConfig({ - channelId: item.channel, - channelName, - channels: ctx.channelsConfig, - }); - if (channelConfig?.allowed === false) return; - } - - const actor = await ctx.resolveUserName(event.user); - const shouldNotify = shouldEmitSlackReactionNotification({ - mode: ctx.reactionMode, - botId: ctx.botUserId, - messageAuthorId: event.item_user ?? undefined, - userId: event.user, - userName: actor?.name ?? undefined, - allowlist: ctx.reactionAllowlist, + const channelLabel = resolveSlackChannelLabel({ + channelId: item.channel, + channelName: channelInfo?.name, }); - if (!shouldNotify) return; - + const actorInfo = event.user ? await ctx.resolveUserName(event.user) : undefined; + const actorLabel = actorInfo?.name ?? event.user; const emojiLabel = event.reaction ?? "emoji"; - const actorLabel = actor?.name ?? event.user; - const channelLabel = channelName - ? `#${normalizeSlackSlug(channelName) || channelName}` - : `#${item.channel}`; const authorInfo = event.item_user ? await ctx.resolveUserName(event.item_user) : undefined; const authorLabel = authorInfo?.name ?? event.item_user; const baseText = `Slack reaction ${action}: :${emojiLabel}: by ${actorLabel} in ${channelLabel} msg ${item.ts}`; @@ -79,13 +51,18 @@ export function registerSlackReactionEvents(params: { ctx: SlackMonitorContext } } }; - ctx.app.event("reaction_added", async ({ event }: SlackEventMiddlewareArgs<"reaction_added">) => { - await handleReactionEvent(event as SlackReactionEvent, "added"); - }); + ctx.app.event( + "reaction_added", + async ({ event, body }: SlackEventMiddlewareArgs<"reaction_added">) => { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; + await handleReactionEvent(event as SlackReactionEvent, "added"); + }, + ); ctx.app.event( "reaction_removed", - async ({ event }: SlackEventMiddlewareArgs<"reaction_removed">) => { + async ({ event, body }: SlackEventMiddlewareArgs<"reaction_removed">) => { + if (ctx.shouldDropMismatchedSlackEvent(body)) return; await handleReactionEvent(event as SlackReactionEvent, "removed"); }, ); diff --git a/src/slack/monitor/provider.ts b/src/slack/monitor/provider.ts index de76eb924..a0a46e949 100644 --- a/src/slack/monitor/provider.ts +++ b/src/slack/monitor/provider.ts @@ -18,6 +18,13 @@ import { registerSlackMonitorSlashCommands } from "./slash.js"; import type { MonitorSlackOpts } from "./types.js"; +function parseApiAppIdFromAppToken(raw?: string) { + const token = raw?.trim(); + if (!token) return undefined; + const match = /^xapp-\d-([a-z0-9]+)-/i.exec(token); + return match?.[1]?.toUpperCase(); +} + export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { const cfg = opts.config ?? loadConfig(); @@ -81,14 +88,23 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { let botUserId = ""; let teamId = ""; + let apiAppId = ""; + const expectedApiAppIdFromAppToken = parseApiAppIdFromAppToken(appToken); try { const auth = await app.client.auth.test({ token: botToken }); botUserId = auth.user_id ?? ""; teamId = auth.team_id ?? ""; + apiAppId = (auth as { api_app_id?: string }).api_app_id ?? ""; } catch { // auth test failing is non-fatal; message handler falls back to regex mentions. } + if (apiAppId && expectedApiAppIdFromAppToken && apiAppId !== expectedApiAppIdFromAppToken) { + runtime.error?.( + `slack token mismatch: bot token api_app_id=${apiAppId} but app token looks like api_app_id=${expectedApiAppIdFromAppToken}`, + ); + } + const ctx = createSlackMonitorContext({ cfg, accountId: account.accountId, @@ -97,6 +113,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { runtime, botUserId, teamId, + apiAppId, historyLimit, sessionScope, mainKey,