← Back to PRs

#19435: fix(slack): properly handle group DM (MPIM) events

by Utkarshbhimte open 2026-02-17 20:05 View on GitHub →
channel: slack size: XS
Summary • Problem: Group DMs (MPIMs) bypass auth gating, mention requirements, and channel config — every message gets processed regardless of allowFrom or requireMention settings • Why it matters: Bots respond to every message in group DMs without authorization checks, causing spam and security gaps • What changed: Extended prepare.ts to treat group DMs (isRoomish) consistently with channels for auth, mention gating, channel config, labels, and ChatType • What did NOT change: Route resolution (peer.kind) and isMentionableGroup still distinguish channels from group DMs. No changes to DM (1:1) handling. Change Type • [x] Bug fix Scope • [x] Integrations User-visible / Behavior Changes • Group DMs now respect allowFrom — unauthorized senders are blocked • Group DMs now respect requireMention — bot only responds when mentioned (if configured) • Group DMs can use per-channel config (systemPrompt, users allowlist, requireMention) • Inbound label shows "Slack group DM" instead of "Slack message" • ChatType is "group" instead of "channel" for group DMs, fixing session routing Security Impact • New permissions/capabilities? No • Secrets/tokens handling changed? No • New/changed network calls? No • Command/tool execution surface changed? No • Data access scope changed? Yes — group DMs now enforce allowFrom, reducing access scope  • Risk: None (tightening, not loosening) Repro + Verification Environment • OS: Linux • Runtime: Node v22 • Integration: Slack Steps 1. Add bot to a Slack group DM 2. Send a message without mentioning the bot 3. Observe bot processes every message (before fix) Expected • Bot only responds when mentioned (if requireMention is set) • Unauthorized senders are blocked Actual (before fix) • Bot processes every message regardless of config Compatibility / Migration • Backward compatible? Yes — existing group DM behavior becomes stricter but follows existing config patterns • Config/env changes? No • Migration needed? No Failure Recovery • Revert this single commit • Watch for: group DMs suddenly not being processed (check groupDmEnabled and allowFrom config) Risks and Mitigations • Risk: Existing group DM conversations stop working if allowFrom is restrictive  • Mitigation: Only applies when allowFrom has entries; open policy still works <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a real security/behavior gap where Slack group DMs (MPIMs) bypassed all authorization and configuration checks — `allowFrom` filtering, `requireMention` gating, and per-channel config — causing the bot to respond to every group DM message unconditionally. The fix extends `isRoomish` (already covering channels and private groups) to include group DMs across the relevant gating points in `prepare.ts`, and correctly sets `chatType` to `"group"` for MPIMs (previously `"channel"`), aligning it with the session-key logic that already distinguished group DMs. Key observations: - The core auth fix (`allowFrom` check, `requireMention`, `channelConfig`, `chatType`) is correct and consistent with how the rest of the codebase treats these message types. - `isGroup: isRoomish` in the ack-reaction gate is a subtle scope change: with the default `"group-mentions"` scope it's harmless (blocked by `isMentionableGroup: isRoom`), but users with `ackReactionScope: "group-all"` will now get ack reactions in group DMs as a new side-effect. - The `!ctx.groupDmEnabled` check in the new `isGroupDm` block is redundant — `isChannelAllowed` already enforces this earlier in the same function — but is harmless. - Import reordering (type imports moved to the top) is a style-only change with no behavioral impact. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge; it tightens access controls for group DMs with no loosening of existing permissions. - The core bug fix is well-targeted and correct. The two noted issues are minor: a redundant guard (cosmetic) and an undocumented side-effect on `ackReactionScope: "group-all"` (edge case). No logic errors or regressions were found in the main auth, mention-gating, or session-routing paths. - No files require special attention beyond the two style-level observations noted in inline comments. <sub>Last reviewed commit: 9c69780</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs