fix(slack): refuse a custom shared-app row at setup start, scope lock barriers, assert conflict code

Author: waleedlatif1

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

@@ -24,8 +24,8 @@ interface Release {
 
 const LATEST: Release = { top: 10, notes: [118, 92] }
 
-/** Every release ends inside the 148px viewBox; the fade carries the timeline past it. */
-const EARLIER: readonly Release[] = [{ top: 70, notes: [104, 126, 80] }] 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
@@ -84,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

@@ -222,14 +222,20 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       locked.resolve(connection.pid)
       await release.promise
     })
-    const holderPid = await locked.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))`
+            sql`SELECT pid FROM pg_stat_activity
+              WHERE ${holderPid} = ANY(pg_blocking_pids(pid)) AND query LIKE '%lock=''slack_org_app''%'`
           )
           expect(waiting).toBeDefined()
         },
@@ -406,9 +412,11 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
     ).resolves.toEqual({ id: installationId })
   })
 
-  it('explains a conflict when a custom app row holds the shared app ID', async () => {
+  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,
@@ -419,12 +427,12 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
       encryptedSigningSecret: secret,
       revision: generateId(),
     })
-    const pending = new URL((await startSetup(groupId)).authorizationUrl)
-    const before = await accountsGroup()
     provideSlackConsent(MEMBER_TEAM)
-    await expect(completeSetup(pending.searchParams.get('state') ?? '')).rejects.toThrow(
-      'belongs to another installation owner'
-    )
+    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 })

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

@@ -10,11 +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 Slack Search's own installation writes
- * (install, rebind, enable, disable, removal), so neither commits against an app or workspace the
- * other changed. Taken 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,
- * and member setup stays valid without one.
+ * 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

@@ -569,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.',
@@ -901,7 +911,7 @@ export async function exchangeAndConfigureSlackManagedUsers(params: {
         )
         .limit(1)
         .for('update')
-      /** A custom row holding the shared app's ID belongs to another owner; never adopt it. */
+      /** 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',