From 586170b8acdc548a7c22619f41ce3aa5f59b9a1c Mon Sep 17 00:00:00 2001 From: Youcef Date: Tue, 4 Aug 2026 21:48:09 +0100 Subject: [PATCH] feat(slack): add allowlist controls for agent access --- src/slack/config.ts | 24 ++++++++ src/slack/events.ts | 24 +++++++- src/slack/index.ts | 1 + test/slack-http-events.test.ts | 6 ++ test/slack-index.integration.test.ts | 83 +++++++++++++++++++++++++--- 5 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/slack/config.ts b/src/slack/config.ts index c8b17930..2b19ed3b 100644 --- a/src/slack/config.ts +++ b/src/slack/config.ts @@ -19,10 +19,31 @@ export interface SlackPluginConfig { maxPrivateChannels?: number; recentMessages?: number; userCacheTtlMs?: number; + allow_dms: boolean; + user_allowlist: string[]; + channel_allowlist: string[]; botIdentity?: { username?: string; icon_emoji?: string }; devIntrospection?: { port: number }; } +function parseBool(raw: string | undefined): boolean | undefined { + if (raw === undefined) return undefined; + const value = raw.trim().toLowerCase(); + if (value === "1" || value === "true" || value === "yes" || value === "on") return true; + if (value === "0" || value === "false" || value === "no" || value === "off") return false; + return undefined; +} + +function parseCsv(raw: string | undefined): string[] { + if (!raw) return []; + const values: string[] = []; + for (const part of raw.split(",")) { + const value = part.replace(/^\s+|\s+$/g, ""); + if (value) values.push(value); + } + return values; +} + export function slackPluginConfigFromEnv(env: Record): SlackPluginConfig | null { const eventsMode = env.SLACK_EVENTS_MODE?.trim() === "http" ? "http" : "socket"; if (!env.SLACK_BOT_TOKEN) return null; @@ -53,6 +74,9 @@ export function slackPluginConfigFromEnv(env: Record ...opt("maxPrivateChannels", num(env.SLACK_MAX_PRIVATE_CHANNELS)), ...opt("recentMessages", num(env.SLACK_RECENT_MESSAGES)), ...opt("userCacheTtlMs", num(env.SLACK_USER_CACHE_TTL_MS)), + allow_dms: parseBool(env.SLACK_ALLOW_DMS) ?? true, + user_allowlist: parseCsv(env.SLACK_USER_ALLOWLIST), + channel_allowlist: parseCsv(env.SLACK_CHANNEL_ALLOWLIST), ...(() => { const identity = botIdentityFromEnv(env); return Object.keys(identity).length ? { botIdentity: identity } : {}; diff --git a/src/slack/events.ts b/src/slack/events.ts index b964e3f7..8486ce0e 100644 --- a/src/slack/events.ts +++ b/src/slack/events.ts @@ -13,8 +13,26 @@ import { import type { AckGate } from "./deferred-ack.ts"; import type { BotIdentity, Directory } from "./directory.ts"; import type { Mirror } from "./mirror.ts"; +import type { SlackPluginConfig } from "./config.ts"; import type { SlackReactionEvent, TurnHandler } from "./turn-handler.ts"; +function canProcessSlackEvent( + config: SlackPluginConfig, + channelType: string | undefined, + userId: string | undefined, + channelId: string | undefined, +): boolean { + if (channelType === "im") { + if (!config.allow_dms) return false; + if (config.user_allowlist.length > 0 && (!userId || !config.user_allowlist.includes(userId))) return false; + return true; + } + if (channelType === "channel" || channelType === "group" || channelType === "mpim") { + if (config.channel_allowlist.length > 0 && (!channelId || !config.channel_allowlist.includes(channelId))) return false; + } + return true; +} + export function registerSlackEvents( app: { event(name: string, handler: (args: any) => Promise): void; @@ -28,15 +46,17 @@ export function registerSlackEvents( deduper: ReturnType; webUiPublicUrl?: string; ensureHeader?: (client: SurfaceHeaderClient, channel: string, scopeId: string, kind: "dm" | "channel") => void; + config: SlackPluginConfig; }, ): void { - const { handler, mirror, directory, ids, deduper } = deps; + const { handler, mirror, directory, ids, deduper, config } = deps; const { dispatch, handleReactionEvent, botHasStakeInThread } = handler; const { mirrorMessageEvent, pushSurfaceEvents } = mirror; const { knownPublicChannels, syncForUnseenGroup, forceDirectorySync } = directory; app.event("app_mention", async ({ event, body, client, context }: any) => { const e = event as any; + if (!canProcessSlackEvent(config, e.channel_type ?? "channel", e.user, e.channel)) return; const key = dedupeKey({ event_id: (body as any)?.event_id, client_msg_id: e.client_msg_id, @@ -101,6 +121,7 @@ export function registerSlackEvents( if (!shouldProcessMessage(m, ids.botUserId, ids.ownBotId)) return; if (m.channel_type === "im") { + if (!canProcessSlackEvent(config, m.channel_type, m.user, m.channel)) return; const key = dedupeKey({ event_id: (body as any)?.event_id, client_msg_id: m.client_msg_id, @@ -126,6 +147,7 @@ export function registerSlackEvents( } if (m.channel_type === "channel" || m.channel_type === "group" || m.channel_type === "mpim") { + if (!canProcessSlackEvent(config, m.channel_type, m.user, m.channel)) return; if (m.channel_type === "mpim" && m.channel) syncForUnseenGroup(client, String(m.channel)); const threadReply = isThreadReply(m); const isMention = mentionsBot(m.text ?? "", ids.botUserId); diff --git a/src/slack/index.ts b/src/slack/index.ts index 6160d655..0818f1b3 100644 --- a/src/slack/index.ts +++ b/src/slack/index.ts @@ -161,6 +161,7 @@ export async function startSlackPlugin( directory, ids, deduper, + config: cfg, ...(cfg.webUiPublicUrl ? { webUiPublicUrl: cfg.webUiPublicUrl } : {}), ensureHeader, }); diff --git a/test/slack-http-events.test.ts b/test/slack-http-events.test.ts index 4bbf6950..24ae578d 100644 --- a/test/slack-http-events.test.ts +++ b/test/slack-http-events.test.ts @@ -12,8 +12,14 @@ test("HTTP events mode trims the configured enum like the deployment secret gate SLACK_EVENTS_MODE: " http ", SLACK_BOT_TOKEN: "xoxb-test", SLACK_SIGNING_SECRET: SECRET, + SLACK_ALLOW_DMS: "false", + SLACK_USER_ALLOWLIST: "U1, U2 ", + SLACK_CHANNEL_ALLOWLIST: "C1,C2", }); assert.equal(config?.eventsMode, "http"); + assert.equal(config?.allow_dms, false); + assert.deepEqual(config?.user_allowlist, ["U1", "U2"]); + assert.deepEqual(config?.channel_allowlist, ["C1", "C2"]); }); function sign(body: string, ts = Math.floor(Date.now() / 1000)): { signature: string; timestamp: string } { diff --git a/test/slack-index.integration.test.ts b/test/slack-index.integration.test.ts index a5bc2a65..46feee04 100644 --- a/test/slack-index.integration.test.ts +++ b/test/slack-index.integration.test.ts @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { mock, test } from "node:test"; import type { SlackCoreClient } from "../src/slack/index.ts"; +import type { SlackPluginConfig } from "../src/slack/config.ts"; import type { TurnResult } from "../src/types.ts"; type Handler = (args: any) => Promise; @@ -301,16 +302,23 @@ async function waitFor(cond: () => boolean, timeoutMs = 2000): Promise { } } -async function fixture(options: { externalParticipants?: boolean; webUiPublicUrl?: string } = {}) { +async function fixture( + options: { externalParticipants?: boolean; webUiPublicUrl?: string; slackConfig?: Partial } = {}, +) { const core = new FakeCore(); core.externalParticipants = options.externalParticipants ?? false; + const slackConfig: SlackPluginConfig = { + botToken: "xoxb-test", + appToken: "xapp-test", + identityEmail: "0", + allow_dms: true, + user_allowlist: [], + channel_allowlist: [], + ...(options.slackConfig ?? {}), + ...(options.webUiPublicUrl ? { webUiPublicUrl: options.webUiPublicUrl } : {}), + }; const started = startSlackPlugin( - { - botToken: "xoxb-test", - appToken: "xapp-test", - identityEmail: "0", - ...(options.webUiPublicUrl ? { webUiPublicUrl: options.webUiPublicUrl } : {}), - }, + slackConfig, core, ); const app = FakeApp.instances.at(-1)!; @@ -342,7 +350,14 @@ test("config is all-or-nothing and numeric tuning fails closed", () => { SLACK_CHANNEL_MEMBERS_TTL_MS: "NaN", SLACK_MAX_PRIVATE_CHANNELS: "10", }); - assert.deepEqual(config, { botToken: "xoxb", appToken: "xapp", maxPrivateChannels: 10 }); + assert.deepEqual(config, { + botToken: "xoxb", + appToken: "xapp", + maxPrivateChannels: 10, + allow_dms: true, + user_allowlist: [], + channel_allowlist: [], + }); }); test("a mid-turn message that STEERS the live run does not post the reply twice", async () => { @@ -538,6 +553,30 @@ test("an external principal is refused in a DM before core sees the text", async } }); +test("a DM allowlist blocks non-pilot users before core sees the text", async () => { + const f = await fixture({ slackConfig: { user_allowlist: ["U2"] } }); + try { + await f.app.emitMessage({ channel: "D1", channel_type: "im", user: "U1", text: "let me in", ts: "102.2" }); + assert.equal(f.core.turns.length, 0); + assert.equal(f.client.posts.length, 0); + assert.equal(f.core.ingests.flat().some((event) => event.text === "let me in"), false); + } finally { + await f.stop(); + } +}); + +test("allow_dms=false blocks direct messages before core sees the text", async () => { + const f = await fixture({ slackConfig: { allow_dms: false } }); + try { + await f.app.emitMessage({ channel: "D1", channel_type: "im", user: "U1", text: "hello?", ts: "102.3" }); + assert.equal(f.core.turns.length, 0); + assert.equal(f.client.posts.length, 0); + assert.equal(f.core.ingests.flat().some((event) => event.text === "hello?"), false); + } finally { + await f.stop(); + } +}); + test("a Slack Connect mention is refused ephemerally and never mirrored", async () => { const f = await fixture(); try { @@ -553,6 +592,34 @@ test("a Slack Connect mention is refused ephemerally and never mirrored", async } }); +test("a channel allowlist blocks mentions before orchestration starts", async () => { + const f = await fixture({ slackConfig: { channel_allowlist: ["C9"] } }); + try { + const event = { channel: "C1", channel_type: "channel", user: "U1", text: "<@UBOT> hello", ts: "103.15" }; + await f.app.emitEvent("app_mention", event); + assert.equal(f.core.turns.length, 0); + assert.equal(f.core.ingests.length, 0); + assert.equal(f.client.posts.length, 0); + assert.equal(f.client.ephemerals.length, 0); + } finally { + await f.stop(); + } +}); + +test("a channel allowlist blocks mpim thread-follow before directory sync or orchestration", async () => { + const f = await fixture({ slackConfig: { channel_allowlist: ["C1"] } }); + try { + f.client.channelsById.set("G9", { id: "G9", name: "", is_member: true, is_private: true, is_mpim: true }); + const listedBefore = f.client.groupListings; + await f.app.emitMessage({ channel: "G9", channel_type: "mpim", user: "U1", text: "also update", ts: "400.1", thread_ts: "300.1" }); + assert.equal(f.core.turns.length, 0); + assert.equal(f.client.groupListings, listedBefore); + assert.equal(f.core.ingests.length, 0); + } finally { + await f.stop(); + } +}); + test("an unreadable channel roster fails closed before core or mirror ingestion", async () => { const f = await fixture(); try {