fix(slack): clean up unsuccessful shared app grants

Author: TheodoreSpeaks

apps/sim/.env.example

@@ -254,6 +254,6 @@ CRON_SECRET=your_cron_secret # Use `openssl rand -hex 32` to generate. Authentic
 # MISTRAL_OCR_QUOTA_GROUPS={"<64-character lowercase key fingerprint>":"organization-id"}
 
 # Official Sim Search Slack app (optional; requires existing Search access)
-# Register the company app with scripts/register-platform-slack-app.ts --search.
+# Register the company app with bun scripts/register-platform-slack-app.ts <APP_ID> --search.
 # SLACK_SEARCH_APP_ID=
 # SLACK_SEARCH_SHARED_APP=false           # Off-production fallback for the global slack-search-shared-app flag

apps/sim/lib/internal/slack/oauth.test.ts

@@ -1,6 +1,10 @@
 /** @vitest-environment node */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { exchangeSlackBotAuthorization } from '@/lib/internal/slack/oauth'
+import {
+  exchangeSlackBotAuthorization,
+  revokeSlackBotAuthorization,
+  validateSlackBotAuthorization,
+} from '@/lib/internal/slack/oauth'
 import { SLACK_SEARCH_SCOPES } from '@/lib/slack-search/constants'
 
 const fetchMock = vi.fn()
@@ -11,9 +15,9 @@ const input = {
   redirectUri: 'https://sim.test/api/knowledge/slack/oauth/callback',
 }
 const grant = {
-  ok: true,
+  ok: true as const,
   app_id: 'A1',
-  token_type: 'bot',
+  token_type: 'bot' as const,
   access_token: 'test-bot-token',
   bot_user_id: 'UBOT',
   scope: SLACK_SEARCH_SCOPES.join(','),
@@ -34,19 +38,48 @@ describe('Slack bot OAuth exchange', () => {
     expect(request.body.get('redirect_uri')).toBe(input.redirectUri)
     expect(request.body.get('code')).toBe('code')
   })
+  it.each([{ token_type: 'user' }, { ok: false, error: 'invalid_client_id' }])(
+    'rejects incompatible or unsuccessful grants: %j',
+    async (change) => {
+      fetchMock.mockResolvedValueOnce(Response.json({ ...grant, ...change }))
+      await expect(exchangeSlackBotAuthorization(input)).rejects.toThrow()
+    }
+  )
+  it('does not expose provider credentials in error messages', async () => {
+    fetchMock.mockResolvedValueOnce(Response.json({ ok: false, error: 'SECRET-DO-NOT-LOG' }))
+    await expect(exchangeSlackBotAuthorization(input)).rejects.toThrow('Slack authorization failed')
+  })
+})
+
+describe('Slack bot grant policy and cleanup', () => {
   it.each([
-    { token_type: 'user' },
     { is_enterprise_install: true },
     { refresh_token: 'refresh' },
     { expires_in: 3600 },
     { scope: 'chat:write' },
-    { ok: false, error: 'invalid_client_id' },
-  ])('rejects incompatible or unsuccessful grants: %j', async (change) => {
-    fetchMock.mockResolvedValueOnce(Response.json({ ...grant, ...change }))
-    await expect(exchangeSlackBotAuthorization(input)).rejects.toThrow()
+  ])('rejects unsupported grants after the caller takes ownership: %j', (change) => {
+    expect(() => validateSlackBotAuthorization({ ...grant, ...change })).toThrow()
   })
-  it('does not expose provider credentials in error messages', async () => {
+  it('accepts the existing indexing bot scope policy', () => {
+    expect(() => validateSlackBotAuthorization(grant)).not.toThrow()
+  })
+  it('requires the additional command scope for shared installs', () => {
+    expect(() =>
+      validateSlackBotAuthorization(grant, [...SLACK_SEARCH_SCOPES, 'commands'])
+    ).toThrow('commands')
+  })
+  it('revokes an unused token through Slack with a bounded request', async () => {
+    fetchMock.mockResolvedValueOnce(Response.json({ ok: true, revoked: true }))
+    await revokeSlackBotAuthorization('unused-token')
+    const [url, request] = fetchMock.mock.calls[0]
+    expect(String(url)).toContain('/api/auth.revoke')
+    expect(request.headers.Authorization).toBe('Bearer unused-token')
+    expect(request.signal).toBeDefined()
+  })
+  it('fails visibly when Slack does not confirm revocation', async () => {
     fetchMock.mockResolvedValueOnce(Response.json({ ok: false, error: 'SECRET-DO-NOT-LOG' }))
-    await expect(exchangeSlackBotAuthorization(input)).rejects.toThrow('Slack authorization failed')
+    await expect(revokeSlackBotAuthorization('unused-token')).rejects.toThrow(
+      'Slack could not revoke'
+    )
   })
 })

apps/sim/lib/internal/slack/oauth.ts

@@ -2,6 +2,7 @@ import { Buffer } from 'node:buffer'
 import { z } from 'zod'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { readResponseJsonWithLimit } from '@/lib/core/utils/stream-limits'
+import { requestSlackApi } from '@/lib/internal/slack/client'
 import { SLACK_SEARCH_SCOPES } from '@/lib/slack-search/constants'
 
 const botGrantSchema = z.object({
@@ -44,19 +45,35 @@ export async function exchangeSlackBotAuthorization(input: {
       'Slack authorization failed. Check the client credentials and install the app again.'
     )
   }
-  const grant = parsed.data
-  if (grant.is_enterprise_install || grant.refresh_token || grant.expires_in) {
+  return parsed.data
+}
+
+/** Runs after exchange so the application can clean up an issued grant if policy rejects it. */
+export function validateSlackBotAuthorization(
+  grant: z.infer<typeof botGrantSchema>,
+  requiredScopes: readonly string[] = SLACK_SEARCH_SCOPES
+) {
+  if (grant.is_enterprise_install || grant.refresh_token || grant.expires_in)
     throw new OrchestrationError(
       'validation',
       'Install the app in one workspace with token rotation disabled.'
     )
-  }
   const scopes = grant.scope.split(',').map((scope) => scope.trim())
-  const missing = SLACK_SEARCH_SCOPES.filter((scope) => !scopes.includes(scope))
+  const missing = requiredScopes.filter((scope) => !scopes.includes(scope))
   if (missing.length)
     throw new OrchestrationError(
       'validation',
       `Reinstall the app with these scopes: ${missing.join(', ')}`
     )
-  return grant
+}
+
+/** Revokes an unused bot grant after failed setup without logging provider credentials. */
+export async function revokeSlackBotAuthorization(accessToken: string) {
+  const response = await requestSlackApi({
+    accessToken,
+    method: 'auth.revoke',
+    signal: AbortSignal.timeout(10_000),
+  })
+  if (response.status !== 200 || response.data.ok !== true || response.data.revoked !== true)
+    throw new OrchestrationError('validation', 'Slack could not revoke the unused setup token')
 }

apps/sim/lib/knowledge/application/slack-search/setup.test.ts

@@ -14,6 +14,9 @@ const m = vi.hoisted(() => ({
   audit: vi.fn(),
   baseUrl: vi.fn(),
   shared: vi.fn(),
+  revoke: vi.fn(),
+  validateGrant: vi.fn(),
+  ensureGroup: vi.fn(),
 }))
 vi.mock('@/lib/slack-search/shared-app', () => ({ readSharedSlackSearchApp: m.shared }))
 vi.mock('@sim/audit', () => ({
@@ -45,7 +48,12 @@ vi.mock('@/lib/slack-search/oauth-state', () => ({
   consumeSlackSearchOAuthAttempt: m.consume,
   storeSlackSearchOAuthAttempt: m.store,
 }))
-vi.mock('@/lib/internal/slack/oauth', () => ({ exchangeSlackBotAuthorization: m.exchange }))
+vi.mock('@/lib/internal/slack/oauth', () => ({
+  exchangeSlackBotAuthorization: m.exchange,
+  revokeSlackBotAuthorization: m.revoke,
+  validateSlackBotAuthorization: m.validateGrant,
+}))
+vi.mock('@/lib/credential-groups/service', () => ({ ensureWorkspaceAccountsGroup: m.ensureGroup }))
 vi.mock('@/lib/credential-groups/organization-slack-app', () => ({
   loadOrganizationSlackMemberApps: async () => [],
   adoptOrganizationSlackMemberApp: vi.fn(),
@@ -87,6 +95,9 @@ const complete = () =>
 beforeEach(() => {
   vi.clearAllMocks()
   m.shared.mockResolvedValue(null)
+  m.revoke.mockResolvedValue(undefined)
+  m.validateGrant.mockReset()
+  m.ensureGroup.mockResolvedValue({ id: 'accounts' })
   m.baseUrl.mockReturnValue('https://sim.test')
   m.membership.mockResolvedValue([{ role: 'admin' }])
   m.rows.mockReset().mockResolvedValue([])
@@ -286,3 +297,130 @@ it('rejects a shared-app callback if the global configuration was disabled or ro
   await expect(complete()).rejects.toThrow()
   expect(m.exchange).not.toHaveBeenCalled()
 })
+
+describe('shared app completion', () => {
+  const sharedApp = { id: 'A1', revision: 'shared-revision', kind: 'shared', organizationId: null }
+  beforeEach(() => {
+    m.shared.mockResolvedValue(sharedApp)
+    m.consume.mockResolvedValue({
+      ...attempt,
+      sharedApp: { id: sharedApp.id, revision: sharedApp.revision },
+    })
+  })
+
+  it('commits the personal app configuration, bot credential and installation in one transaction', async () => {
+    m.rows
+      .mockResolvedValueOnce([sharedApp])
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([
+        { id: 'accounts', options: [], encryptedProviderConfiguration: null },
+      ])
+    await expect(complete()).resolves.toEqual({ organizationId: 'org1' })
+    expect(db.transaction).toHaveBeenCalledOnce()
+    expect(m.ensureGroup).toHaveBeenCalledWith(
+      { kind: 'organization', organizationId: 'org1' },
+      'admin',
+      undefined,
+      expect.objectContaining({ insert: expect.any(Function) })
+    )
+    const group = m.set.mock.calls[0][0]
+    expect(group.options).toEqual([
+      expect.objectContaining({
+        provider: 'slack',
+        authorizationAppId: 'slack:A1:T1',
+        status: 'active',
+        requiredScopes: expect.arrayContaining([
+          'channels:history',
+          'groups:history',
+          'im:history',
+          'mpim:history',
+          'users:read.email',
+        ]),
+      }),
+    ])
+    const configuration = JSON.parse(
+      group.encryptedProviderConfiguration.slice('encrypted:'.length)
+    )
+    expect(configuration.slack).toMatchObject({
+      source: 'slack_app',
+      appId: 'A1',
+      teamId: 'T1',
+      scopes: group.options[0].requiredScopes,
+    })
+    expect(configuration.slack).not.toHaveProperty('clientSecret')
+    const rows = m.values.mock.calls.map(([value]) => value)
+    expect(rows).toHaveLength(2)
+    expect(rows[0]).toMatchObject({
+      organizationId: 'org1',
+      workspaceId: null,
+      type: 'service_account',
+      slackAppId: 'A1',
+    })
+    expect(rows[1]).toMatchObject({
+      organizationId: 'org1',
+      credentialId: rows[0].id,
+      slackAppId: 'A1',
+      appId: 'A1',
+      teamId: 'T1',
+      enabled: true,
+    })
+    expect(m.verify).toHaveBeenCalledTimes(2)
+    expect(m.revoke).not.toHaveBeenCalled()
+    expect(m.audit).toHaveBeenCalledOnce()
+  })
+
+  it('revokes an unused shared bot grant after a conflicting workspace binding', async () => {
+    m.rows
+      .mockResolvedValueOnce([sharedApp])
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([{ id: 'other-app' }])
+    await expect(complete()).rejects.toThrow('already has an active Search installation')
+    expect(m.revoke).toHaveBeenCalledWith('bot-token')
+    expect(m.values).not.toHaveBeenCalled()
+    expect(m.audit).not.toHaveBeenCalled()
+  })
+
+  it('revokes an unused shared grant after a database write fails', async () => {
+    m.rows
+      .mockResolvedValueOnce([sharedApp])
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce([
+        { id: 'accounts', options: [], encryptedProviderConfiguration: null },
+      ])
+    m.values.mockImplementationOnce(() => {
+      throw new Error('write failed')
+    })
+    await expect(complete()).rejects.toThrow('write failed')
+    expect(m.revoke).toHaveBeenCalledWith('bot-token')
+    expect(m.audit).not.toHaveBeenCalled()
+  })
+
+  it('never revokes a bot with an existing installation when the initiating admin loses access', async () => {
+    m.verify.mockImplementationOnce(async () => {
+      m.membership.mockResolvedValue([{ role: 'member' }])
+      return identity
+    })
+    m.rows.mockResolvedValueOnce([{ id: 'existing-installation' }])
+    await expect(complete()).rejects.toThrow('administrator')
+    expect(m.revoke).not.toHaveBeenCalled()
+    expect(m.values).not.toHaveBeenCalled()
+  })
+
+  it('revokes a shared grant rejected by scope or token-rotation policy', async () => {
+    m.validateGrant.mockImplementationOnce(() => {
+      throw new Error('unsupported grant')
+    })
+    await expect(complete()).rejects.toThrow('unsupported grant')
+    expect(m.revoke).toHaveBeenCalledWith('bot-token')
+    expect(m.values).not.toHaveBeenCalled()
+  })
+
+  it('surfaces cleanup failure with a concrete recovery step', async () => {
+    m.verify.mockRejectedValueOnce(new Error('invalid bot'))
+    m.revoke.mockRejectedValueOnce(new Error('provider failed'))
+    await expect(complete()).rejects.toThrow('Remove the unused app in Slack before retrying')
+    expect(m.audit).not.toHaveBeenCalled()
+  })
+})