From 7820925c76c0ebd1d1515843de121c42331ad257 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 28 May 2026 15:25:40 -0700 Subject: [PATCH 1/3] feat(slack): scope private channel visibility to installing user --- .../sim/app/api/tools/slack/channels/route.ts | 177 +++++++++++++++--- apps/sim/lib/auth/auth.ts | 29 ++- 2 files changed, 171 insertions(+), 35 deletions(-) diff --git a/apps/sim/app/api/tools/slack/channels/route.ts b/apps/sim/app/api/tools/slack/channels/route.ts index 6603c36d867..66b6dc4d0af 100644 --- a/apps/sim/app/api/tools/slack/channels/route.ts +++ b/apps/sim/app/api/tools/slack/channels/route.ts @@ -1,4 +1,7 @@ +import { db } from '@sim/db' +import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { slackChannelsSelectorContract } from '@/lib/api/contracts/selectors/slack' import { parseRequest } from '@/lib/api/server' @@ -20,6 +23,21 @@ interface SlackChannel { is_member: boolean } +/** + * Extracts the installing user's Slack id from credentials connected after the + * privacy fix, which `auth.ts` tags with a `usr_` marker + * (`${teamId}-usr_${installerUserId}-${uuid}`). Legacy credentials encode the + * bot id with no marker and return null, so the caller keeps the existing + * `is_member` filter — no regression. + */ +const SCOPED_USER_ID_PATTERN = + /-usr_([UWB][A-Z0-9]+)-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + +function parseScopedSlackUserId(accountId: string): string | null { + const match = SCOPED_USER_ID_PATTERN.exec(accountId) + return match ? match[1] : null +} + export const POST = withRouteHandler(async (request: NextRequest) => { try { const requestId = generateRequestId() @@ -32,6 +50,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { let accessToken: string let isBotToken = false + let scopedUserId: string | null = null if (credential.startsWith('xoxb-')) { accessToken = credential @@ -65,11 +84,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } accessToken = resolvedToken logger.info('Using OAuth token for Slack API') + + if (authz.resolvedCredentialId) { + const [accountRow] = await db + .select({ accountId: account.accountId }) + .from(account) + .where(eq(account.id, authz.resolvedCredentialId)) + .limit(1) + if (accountRow) { + scopedUserId = parseScopedSlackUserId(accountRow.accountId) + } + } } - let data + let data: SlackConversationsResult try { data = await fetchSlackChannels(accessToken, true) + if (data.truncated) { + logger.warn('conversations.list hit pagination cap; channel list may be incomplete') + } logger.info('Successfully fetched channels including private channels') } catch (error) { if (isBotToken) { @@ -96,17 +129,49 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } + /** + * Slack Marketplace privacy: a private channel may only be shown to a user + * whose own Slack account is a member, even when the bot has been invited. + * `users.conversations?user=` returns the channels the bot AND that user + * share, giving us the allowed set. Public channels are never restricted. + * Without a scoped user id (legacy credentials), fall back to bot membership. + */ + let allowedPrivateChannelIds: Set | null = null + if (scopedUserId) { + try { + const userPrivate = await fetchUserPrivateChannels(accessToken, scopedUserId) + allowedPrivateChannelIds = new Set(userPrivate.channels.map((c) => c.id)) + if (userPrivate.truncated) { + logger.warn( + 'users.conversations hit pagination cap; some private channels the user belongs to may be hidden', + { scopedUserId } + ) + } + logger.info('Scoped private channels to installing user membership', { + scopedUserId, + allowedCount: allowedPrivateChannelIds.size, + }) + } catch (scopeError) { + // Fail closed: if membership can't be verified, hide all private channels. + logger.warn('Failed to scope private channels to user, hiding all private channels', { + error: (scopeError as Error).message, + }) + allowedPrivateChannelIds = new Set() + } + } + const channels = (data.channels || []) .filter((channel: SlackChannel) => { - const canAccess = !channel.is_archived && (channel.is_member || !channel.is_private) + if (channel.is_archived) return false - if (!canAccess) { - logger.debug( - `Filtering out channel: ${channel.name} (archived: ${channel.is_archived}, private: ${channel.is_private}, member: ${channel.is_member})` - ) + if (channel.is_private) { + if (allowedPrivateChannelIds) { + return allowedPrivateChannelIds.has(channel.id) + } + return channel.is_member } - return canAccess + return true }) .filter((channel: SlackChannel) => { const validation = validateAlphanumericId(channel.id, 'channelId', 50) @@ -141,6 +206,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { private: channels.filter((c: { isPrivate: boolean }) => c.isPrivate).length, public: channels.filter((c: { isPrivate: boolean }) => !c.isPrivate).length, tokenType: isBotToken ? 'bot_token' : 'oauth', + userScoped: !!scopedUserId, }) return NextResponse.json({ channels }) } catch (error) { @@ -152,35 +218,86 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } }) -async function fetchSlackChannels(accessToken: string, includePrivate = true) { - const url = new URL('https://slack.com/api/conversations.list') +const SLACK_PAGE_LIMIT = 200 +const SLACK_MAX_PAGES = 10 - if (includePrivate) { - url.searchParams.append('types', 'public_channel,private_channel') - } else { - url.searchParams.append('types', 'public_channel') - } +interface SlackConversationsResult { + channels: SlackChannel[] + truncated: boolean +} - url.searchParams.append('exclude_archived', 'true') - url.searchParams.append('limit', '200') +/** + * Lists Slack conversations, following `response_metadata.next_cursor` so the + * full set is returned. Bounded by `SLACK_MAX_PAGES`; sets `truncated` rather + * than silently dropping channels when the cap is hit. + */ +async function fetchAllConversations( + method: 'conversations.list' | 'users.conversations', + accessToken: string, + params: Record +): Promise { + const channels: SlackChannel[] = [] + let cursor: string | undefined + let truncated = false - const response = await fetch(url.toString(), { - method: 'GET', - headers: { - Authorization: `Bearer ${accessToken}`, - 'Content-Type': 'application/json', - }, - }) + for (let page = 0; page < SLACK_MAX_PAGES; page++) { + const url = new URL(`https://slack.com/api/${method}`) + for (const [key, value] of Object.entries(params)) { + url.searchParams.append(key, value) + } + url.searchParams.append('limit', String(SLACK_PAGE_LIMIT)) + if (cursor) { + url.searchParams.append('cursor', cursor) + } - if (!response.ok) { - throw new Error(`Slack API error: ${response.status} ${response.statusText}`) - } + const response = await fetch(url.toString(), { + method: 'GET', + headers: { Authorization: `Bearer ${accessToken}` }, + }) + + if (!response.ok) { + throw new Error(`Slack API error: ${response.status} ${response.statusText}`) + } + + const data = await response.json() + + if (!data.ok) { + throw new Error(data.error || `Failed to fetch ${method}`) + } - const data = await response.json() + if (Array.isArray(data.channels)) { + channels.push(...data.channels) + } - if (!data.ok) { - throw new Error(data.error || 'Failed to fetch channels') + cursor = data.response_metadata?.next_cursor?.trim() || undefined + if (!cursor) { + return { channels, truncated } + } + if (page === SLACK_MAX_PAGES - 1) { + truncated = true + } } - return data + return { channels, truncated } +} + +async function fetchSlackChannels( + accessToken: string, + includePrivate = true +): Promise { + return fetchAllConversations('conversations.list', accessToken, { + types: includePrivate ? 'public_channel,private_channel' : 'public_channel', + exclude_archived: 'true', + }) +} + +async function fetchUserPrivateChannels( + accessToken: string, + userId: string +): Promise { + return fetchAllConversations('users.conversations', accessToken, { + user: userId, + types: 'private_channel', + exclude_archived: 'true', + }) } diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 2943fb31fd7..3ea2384a2d2 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -2532,17 +2532,36 @@ export const auth = betterAuth({ } const teamId = data.team_id || 'unknown' - const userId = data.user_id || data.bot_id || 'bot' const teamName = data.team || 'Slack Workspace' - const uniqueId = `${teamId}-${userId}` - - logger.info('Slack credential identifier', { teamId, userId, uniqueId, teamName }) + /** + * Tag the accountId with the installing user's Slack id (from the OAuth + * v2 `authed_user.id`, preserved on `tokens.raw`) behind a `usr_` marker. + * The channels selector uses it to scope private-channel visibility to + * the installer's own Slack membership, per Slack Marketplace rules. The + * marker disambiguates it from a legacy bot id (same `U.../B...` shape); + * absent it, we keep the legacy format and today's behavior. + */ + const authedUser = tokens.raw?.authed_user as { id?: string } | undefined + const installerUserId = authedUser?.id + const userSegment = installerUserId + ? `usr_${installerUserId}` + : data.user_id || data.bot_id || 'bot' + + const uniqueId = `${teamId}-${userSegment}` + + logger.info('Slack credential identifier', { + teamId, + userSegment, + uniqueId, + teamName, + hasInstallerId: !!installerUserId, + }) return { id: `${uniqueId}-${generateId()}`, name: teamName, - email: `${teamId}-${userId}@slack.bot`, + email: `${uniqueId}@slack.bot`, emailVerified: false, createdAt: new Date(), updatedAt: new Date(), From be61b2d9b794027e2884d67bdf8c355f7648d120 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 28 May 2026 15:37:56 -0700 Subject: [PATCH 2/3] improvement(slack): warn on usr_ marker parse miss, drop dead bot prefix --- apps/sim/app/api/tools/slack/channels/route.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/tools/slack/channels/route.ts b/apps/sim/app/api/tools/slack/channels/route.ts index 66b6dc4d0af..934895dd1f5 100644 --- a/apps/sim/app/api/tools/slack/channels/route.ts +++ b/apps/sim/app/api/tools/slack/channels/route.ts @@ -31,11 +31,20 @@ interface SlackChannel { * `is_member` filter — no regression. */ const SCOPED_USER_ID_PATTERN = - /-usr_([UWB][A-Z0-9]+)-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + /-usr_([UW][A-Z0-9]+)-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i function parseScopedSlackUserId(accountId: string): string | null { const match = SCOPED_USER_ID_PATTERN.exec(accountId) - return match ? match[1] : null + if (match) return match[1] + // A marker is present but the id didn't parse — surface it instead of + // silently dropping to the bot `is_member` filter, which would bypass the + // installer-scoped privacy check without any signal. + if (accountId.includes('-usr_')) { + logger.warn('Slack accountId carries usr_ marker but did not parse; using is_member fallback', { + accountId, + }) + } + return null } export const POST = withRouteHandler(async (request: NextRequest) => { From 0b28afee57e0fdce51efb4b8f8cc43448496a44e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 28 May 2026 15:42:09 -0700 Subject: [PATCH 3/3] chore(slack): trim verbose comment --- apps/sim/app/api/tools/slack/channels/route.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/tools/slack/channels/route.ts b/apps/sim/app/api/tools/slack/channels/route.ts index 934895dd1f5..f7e2c2a3d90 100644 --- a/apps/sim/app/api/tools/slack/channels/route.ts +++ b/apps/sim/app/api/tools/slack/channels/route.ts @@ -36,9 +36,8 @@ const SCOPED_USER_ID_PATTERN = function parseScopedSlackUserId(accountId: string): string | null { const match = SCOPED_USER_ID_PATTERN.exec(accountId) if (match) return match[1] - // A marker is present but the id didn't parse — surface it instead of - // silently dropping to the bot `is_member` filter, which would bypass the - // installer-scoped privacy check without any signal. + // Marker present but unparseable — surface it rather than silently falling + // back to the bot `is_member` filter and bypassing the privacy scope. if (accountId.includes('-usr_')) { logger.warn('Slack accountId carries usr_ marker but did not parse; using is_member fallback', { accountId,