diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/changelog-empty-state.tsx b/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/changelog-empty-state.tsx index e055fe96a1c..740fc94b4b6 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/changelog-empty-state.tsx +++ b/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() { - {EARLIER.map((release) => ( - - - - - - - - ))} + + + + + ) } diff --git a/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts b/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts index 51ba37bc717..de45d377b5c 100644 --- a/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts +++ b/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( + contend: () => Promise, + holdWith: (tx: DbTransaction) => Promise = async () => undefined + ): Promise { + const locked = createDeferred() + const release = createDeferred() + 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() - const release = createDeferred() - 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 () => { diff --git a/apps/sim/lib/credential-groups/organization-slack-app.ts b/apps/sim/lib/credential-groups/organization-slack-app.ts index a2995270721..b9c0124af29 100644 --- a/apps/sim/lib/credential-groups/organization-slack-app.ts +++ b/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}`) diff --git a/apps/sim/lib/credential-groups/slack-managed-users.ts b/apps/sim/lib/credential-groups/slack-managed-users.ts index ebaf30699f2..b9649f75429 100644 --- a/apps/sim/lib/credential-groups/slack-managed-users.ts +++ b/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 diff --git a/apps/sim/lib/knowledge/application/slack-search/installations.ts b/apps/sim/lib/knowledge/application/slack-search/installations.ts index a7abf602955..6c1c64cbe13 100644 --- a/apps/sim/lib/knowledge/application/slack-search/installations.ts +++ b/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 },