fix(slack): serialize Search bot removal with member setup, refuse a custom row on the shared app ID

apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/changelog-empty-state.tsx

@@ -24,10 +24,8 @@ interface Release {
 
 const LATEST: Release = { top: 10, notes: [118, 92] }
 
-const EARLIER: readonly Release[] = [
-  { top: 70, notes: [104, 126, 80] },
-  { top: 142, notes: [96, 112] },
-] as const
+/** Ends inside the 148px viewBox; the fade carries the timeline past it. */
+const EARLIER: Release = { top: 70, notes: [104, 126, 80] }
 
 const SPINE_X = 24
 const NOTE_PITCH = 12
@@ -86,15 +84,11 @@ function ChangelogGraphic() {
       <rect x='92' y={LATEST.top + 6.5} width='34' height='4' rx='2' fill={INK.detail} />
       <ReleaseNotes release={LATEST} />
 
-      {EARLIER.map((release) => (
-        <g key={release.top}>
-          <circle cx={SPINE_X} cy={release.top + 8} r='4' fill={INK.title} />
-          <rect x='42' y={release.top + 2} width='40' height='13' rx='6.5' fill={INK.detail} />
-          <rect x='50' y={release.top + 6.5} width='24' height='4' rx='2' fill={INK.title} />
-          <rect x='92' y={release.top + 6.5} width='34' height='4' rx='2' fill={INK.detail} />
-          <ReleaseNotes release={release} />
-        </g>
-      ))}
+      <circle cx={SPINE_X} cy={EARLIER.top + 8} r='4' fill={INK.title} />
+      <rect x='42' y={EARLIER.top + 2} width='40' height='13' rx='6.5' fill={INK.detail} />
+      <rect x='50' y={EARLIER.top + 6.5} width='24' height='4' rx='2' fill={INK.title} />
+      <rect x='92' y={EARLIER.top + 6.5} width='34' height='4' rx='2' fill={INK.detail} />
+      <ReleaseNotes release={EARLIER} />
     </svg>
   )
 }

apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts

@@ -44,9 +44,12 @@ import { SLACK_SEARCH_USER_SCOPES } from '@/lib/credential-groups/slack-managed-
 import { slackCredentialGroupProviderAdapter } from '@/lib/credential-groups/slack-provider'
 import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts'
 import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth'
-import type { DbOrTx } from '@/lib/db/types'
+import type { DbOrTx, DbTransaction } from '@/lib/db/types'
 import { approveSearchIntegration } from '@/lib/knowledge/application/search-integrations'
-import { listSlackSearchInstallations } from '@/lib/knowledge/application/slack-search/installations'
+import {
+  listSlackSearchInstallations,
+  removeSlackSearchInstallation,
+} from '@/lib/knowledge/application/slack-search/installations'
 import { revokeSlackSearchAccess } from '@/lib/knowledge/application/slack-search/lifecycle'
 import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema'
 import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env'
@@ -181,8 +184,9 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       createdBy: ids.owner,
       encryptedServiceAccountKey: secret,
     })
+    const installationId = generateId()
     await executor.insert(slackSearchInstallation).values({
-      id: generateId(),
+      id: installationId,
       organizationId: ids.organization,
       credentialId: botId,
       appId,
@@ -193,6 +197,7 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       credentialVersion: generateId(),
       revision: generateId(),
     })
+    return installationId
   }
 
   async function accountsGroup() {
@@ -203,6 +208,46 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
     return group
   }
 
+  /** Holds the Slack organization lock until `contend` is observed waiting on it, then releases. */
+  async function whileOrganizationSlackAppLocked<T>(
+    contend: () => Promise<T>,
+    holdWith: (tx: DbTransaction) => Promise<unknown> = async () => undefined
+  ): Promise<T> {
+    const locked = createDeferred<number>()
+    const release = createDeferred<void>()
+    const holder = db.transaction(async (tx) => {
+      await lockOrganizationSlackApp(tx, ids.organization)
+      await holdWith(tx)
+      const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
+      locked.resolve(connection.pid)
+      await release.promise
+    })
+    const holderPid = await Promise.race([
+      locked.promise,
+      holder.then(() => {
+        throw new Error('The lock holder finished before holding the lock')
+      }),
+    ])
+    const contender = contend()
+    contender.catch(() => undefined)
+    try {
+      await vi.waitFor(
+        async () => {
+          const [waiting] = await db.execute<{ pid: number }>(
+            sql`SELECT pid FROM pg_stat_activity
+              WHERE ${holderPid} = ANY(pg_blocking_pids(pid)) AND query LIKE '%lock=''slack_org_app''%'`
+          )
+          expect(waiting).toBeDefined()
+        },
+        { timeout: 5_000 }
+      )
+    } finally {
+      release.resolve()
+      await holder
+    }
+    return contender
+  }
+
   function provideSlackConsent(teamId: string) {
     restoreSlackHttp?.()
     const spy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => {
@@ -345,34 +390,52 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
     const pending = new URL((await startSetup(groupId)).authorizationUrl)
     const before = await accountsGroup()
     provideSlackConsent(MEMBER_TEAM)
-    const locked = createDeferred<number>()
-    const release = createDeferred<void>()
-    const install = db.transaction(async (tx) => {
-      await lockOrganizationSlackApp(tx, ids.organization)
-      await installBot(CUSTOM_APP_ID, OTHER_TEAM, tx)
-      const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
-      locked.resolve(connection.pid)
-      await release.promise
-    })
-    const installPid = await locked.promise
-    const completion = completeSetup(pending.searchParams.get('state') ?? '')
-    completion.catch(() => undefined)
-    try {
-      await vi.waitFor(
-        async () => {
-          const [waiting] = await db.execute<{ pid: number }>(
-            sql`SELECT pid FROM pg_stat_activity WHERE ${installPid} = ANY(pg_blocking_pids(pid))`
-          )
-          expect(waiting).toBeDefined()
-        },
-        { timeout: 5_000 }
+    await expect(
+      whileOrganizationSlackAppLocked(
+        () => completeSetup(pending.searchParams.get('state') ?? ''),
+        (tx) => installBot(CUSTOM_APP_ID, OTHER_TEAM, tx)
       )
-    } finally {
-      release.resolve()
-      await install
-    }
-    await expect(completion).rejects.toThrow('installations changed during authorization')
+    ).rejects.toThrow('installations changed during authorization')
+    expect(await accountsGroup()).toEqual(before)
+  })
+
+  it('waits on the Slack organization lock before removing a Search bot', async () => {
+    await approveSlack()
+    const installationId = await installBot(SHARED_APP_ID, MEMBER_TEAM)
+    await expect(
+      whileOrganizationSlackAppLocked(() =>
+        removeSlackSearchInstallation.execute({
+          principal: principal(),
+          input: { organizationId: ids.organization, installationId },
+        })
+      )
+    ).resolves.toEqual({ id: installationId })
+  })
+
+  it('refuses member setup when a custom app row holds the shared app ID', async () => {
+    const groupId = (await approveSlack()).memberAccounts?.groupId
+    if (!groupId) throw new Error('Approval did not provision organization accounts')
+    const pending = new URL((await startSetup(groupId)).authorizationUrl)
+    const before = await accountsGroup()
+    const secret = (await encryptSecret('fixture-custom')).encrypted
+    await db.insert(slackApp).values({
+      id: SHARED_APP_ID,
+      kind: 'custom',
+      organizationId: ids.organization,
+      clientId: 'fixture-custom-client',
+      encryptedClientSecret: secret,
+      encryptedSigningSecret: secret,
+      revision: generateId(),
+    })
+    provideSlackConsent(MEMBER_TEAM)
+    await expect(completeSetup(pending.searchParams.get('state') ?? '')).rejects.toMatchObject({
+      code: 'conflict',
+      message: 'This Slack app belongs to another installation owner',
+    })
+    await expect(startSetup(groupId)).rejects.toThrow('belongs to another installation owner')
     expect(await accountsGroup()).toEqual(before)
+    const [row] = await db.select().from(slackApp).where(eq(slackApp.id, SHARED_APP_ID))
+    expect(row).toMatchObject({ kind: 'custom', organizationId: ids.organization })
   })
 
   it('reports a custom-app member setup and keeps it off the shared-app path', async () => {

apps/sim/lib/credential-groups/organization-slack-app.ts

@@ -10,8 +10,11 @@ import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
 import type { DbOrTx, DbTransaction } from '@/lib/db/types'
 
 /**
- * Serializes an organization's Slack member setup with its Search bot installs, so each sees the
- * other's committed app and workspace. Taken before any Slack app or workspace lock.
+ * Serializes an organization's Slack member setup with Slack Search's installation writes, so a
+ * bot can never be added or rebound under a member setup that already checked the installations.
+ * Install, rebind, enable, disable and admin removal all take it, first, before any Slack app or
+ * workspace lock. Slack revocations, which must never wait on setup, and the cascade from
+ * deleting a bot credential skip it: they only withdraw a bot, which member setup tolerates.
  */
 export async function lockOrganizationSlackApp(tx: DbTransaction, organizationId: string) {
   await acquireAdvisoryXactLock(tx, 'slack_org_app', `slack-org-app:${organizationId}`)

apps/sim/lib/credential-groups/slack-managed-users.ts

@@ -16,6 +16,7 @@ import { isRecordLike } from '@sim/utils/object'
 import { compareStrings } from '@sim/utils/string'
 import { and, eq, inArray, isNull, or } from 'drizzle-orm'
 import { getRedisClient } from '@/lib/core/config/redis'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { resourceScopeFields, resourceScopeFromOwner } from '@/lib/core/resource-scope'
 import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
@@ -496,7 +497,8 @@ export function getSlackManagedUsersRedirectUri(): string {
 
 /**
  * Names an organization's Search installation set. Installing, rebinding, enabling, disabling,
- * and removing a bot each rotate or drop a row's revision.
+ * and removing a bot each rotate or drop a row's revision. Writes that can add a bot all hold the
+ * Slack organization lock, so a match under that lock means none landed during authorization.
  */
 async function organizationInstallationsRevision(executor: DbOrTx, organizationId: string) {
   const rows = await executor
@@ -567,6 +569,16 @@ async function resolveSharedSlackMemberApp(
       'Set up this organization’s Slack app first.',
       'invalid_response'
     )
+  const [appRow] = await db
+    .select({ kind: slackApp.kind, organizationId: slackApp.organizationId })
+    .from(slackApp)
+    .where(eq(slackApp.id, app.id))
+    .limit(1)
+  if (appRow && (appRow.kind !== 'shared' || appRow.organizationId !== null))
+    throw new SlackManagedUsersError(
+      'This Slack app belongs to another installation owner.',
+      'invalid_response'
+    )
   if (installation)
     throw new SlackManagedUsersError(
       'Select the organization’s installed Slack app.',
@@ -899,6 +911,12 @@ export async function exchangeAndConfigureSlackManagedUsers(params: {
         )
         .limit(1)
         .for('update')
+      /** Start refuses a custom row holding the shared app's ID; this covers one added since. */
+      if (sharedApp && app?.kind !== 'shared')
+        throw new OrchestrationError(
+          'conflict',
+          'This Slack app belongs to another installation owner'
+        )
       if (app?.kind === 'shared')
         await requireSlackSearchAppAvailable(app.id, params.attempt.organizationId)
       const resolved = app ? await resolveSlackAppCredentials(app) : null

apps/sim/lib/knowledge/application/slack-search/installations.ts

@@ -271,15 +271,19 @@ export const removeSlackSearchInstallation = defineAuthorizedKnowledgeUseCase({
   operation: knowledgeOperations.removeSlackInstallation,
   resolveContext: ({ input }: { input: RemoveInput }) => resolveKnowledgeOrganizationContext(input),
   async execute({ input, context }) {
-    const [removed] = await db
-      .delete(slackSearchInstallation)
-      .where(
-        and(
-          eq(slackSearchInstallation.id, input.installationId),
-          eq(slackSearchInstallation.organizationId, context.organizationId)
+    const removed = await db.transaction(async (tx) => {
+      await lockOrganizationSlackApp(tx, context.organizationId)
+      const [row] = await tx
+        .delete(slackSearchInstallation)
+        .where(
+          and(
+            eq(slackSearchInstallation.id, input.installationId),
+            eq(slackSearchInstallation.organizationId, context.organizationId)
+          )
         )
-      )
-      .returning({ id: slackSearchInstallation.id })
+        .returning({ id: slackSearchInstallation.id })
+      return row
+    })
     if (!removed) throw new OrchestrationError('not_found', 'Slack Search installation not found')
     return removed
   },