diff --git a/apps/sim/app/api/users/me/deletion/route.test.ts b/apps/sim/app/api/users/me/deletion/route.test.ts new file mode 100644 index 00000000000..7369ad3bed4 --- /dev/null +++ b/apps/sim/app/api/users/me/deletion/route.test.ts @@ -0,0 +1,81 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + defineInternalJsonRoute: vi.fn(() => vi.fn()), + signOut: vi.fn(), +})) + +vi.mock('@/lib/api/server/routes', () => ({ + defineInternalJsonRoute: mocks.defineInternalJsonRoute, + internalOrchestrationErrorPolicy: { project: vi.fn(), unhandled: vi.fn() }, + internalRateLimits: { none: vi.fn(() => ({ kind: 'none' })) }, + internalSessionAuth: { authenticate: vi.fn() }, +})) + +vi.mock('@/lib/auth', () => ({ + auth: { api: { signOut: mocks.signOut } }, + getSession: vi.fn(), +})) + +vi.mock('@/lib/users/application/delete-account', () => ({ + deleteAccountUseCase: { execute: vi.fn() }, + previewAccountDeletionUseCase: { execute: vi.fn() }, +})) + +vi.mock('@/lib/users/application/operations', () => ({ + userAccountOperations: { delete: { id: 'user.delete' }, previewDeletion: { id: 'user.preview' } }, +})) + +import '@/app/api/users/me/deletion/route' + +type RouteOptions = { + finalizeResponse?: (args: { request: Request }) => Promise<{ headers?: HeadersInit }> +} + +/** Captured at import time; the route registers itself once when the module loads. */ +const routeOptions = mocks.defineInternalJsonRoute.mock.calls.map((call) => call[0] as RouteOptions) + +describe('POST /api/users/me/deletion', () => { + beforeEach(() => { + mocks.signOut.mockReset() + }) + + /** + * The session row is deleted with the account, but the signed cookie cache + * still authenticates the browser for its TTL; the response must clear it. + */ + it('clears the session cookies on the deletion response', async () => { + const options = routeOptions.find( + (candidate) => typeof candidate.finalizeResponse === 'function' + ) + expect(options?.finalizeResponse).toBeDefined() + + const cleared = new Headers([['set-cookie', 'better-auth.session_token=; Max-Age=0']]) + mocks.signOut.mockResolvedValue({ headers: cleared, response: { success: true } }) + const request = new Request('http://localhost/api/users/me/deletion', { + method: 'POST', + headers: { cookie: 'better-auth.session_token=abc' }, + }) + + const finalization = await options!.finalizeResponse!({ request }) + + expect(mocks.signOut).toHaveBeenCalledWith({ headers: request.headers, returnHeaders: true }) + expect(finalization.headers).toBe(cleared) + }) + + it('never fails a completed deletion because the cookies could not be cleared', async () => { + const options = routeOptions.find( + (candidate) => typeof candidate.finalizeResponse === 'function' + ) + mocks.signOut.mockRejectedValue(new Error('sign-out unavailable')) + + await expect( + options!.finalizeResponse!({ + request: new Request('http://localhost/api/users/me/deletion', { method: 'POST' }), + }) + ).resolves.toEqual({}) + }) +}) diff --git a/apps/sim/app/api/users/me/deletion/route.ts b/apps/sim/app/api/users/me/deletion/route.ts index ebb8b64aca7..2b148f889e0 100644 --- a/apps/sim/app/api/users/me/deletion/route.ts +++ b/apps/sim/app/api/users/me/deletion/route.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { deleteAccountContract, getAccountDeletionPlanContract } from '@/lib/api/contracts' import { defineInternalJsonRoute, @@ -5,12 +6,15 @@ import { internalRateLimits, internalSessionAuth, } from '@/lib/api/server/routes' +import { auth } from '@/lib/auth' import { deleteAccountUseCase, previewAccountDeletionUseCase, } from '@/lib/users/application/delete-account' import { userAccountOperations } from '@/lib/users/application/operations' +const logger = createLogger('AccountDeletionRoute') + export const dynamic = 'force-dynamic' export const GET = defineInternalJsonRoute({ @@ -39,4 +43,19 @@ export const POST = defineInternalJsonRoute({ mapInput: ({ body }) => ({ confirmEmail: body.confirmEmail }), useCase: deleteAccountUseCase, present: () => ({ success: true as const }), + /** + * The session row is gone, but the signed cookie cache authenticates this + * browser for up to five more minutes. Clear the cookies on the deletion + * response itself so nothing the page does afterwards can carry them. + */ + finalizeResponse: async ({ request }) => { + try { + const { headers } = await auth.api.signOut({ headers: request.headers, returnHeaders: true }) + return { headers } + } catch (error) { + /** The account is gone either way; the client's own sign-out and full reload still drop the cookie. */ + logger.warn('Could not clear session cookies after account deletion', { error }) + return {} + } + }, }) diff --git a/apps/sim/app/api/workspaces/route.test.ts b/apps/sim/app/api/workspaces/route.test.ts index 54fca6aa265..2ec4cedd881 100644 --- a/apps/sim/app/api/workspaces/route.test.ts +++ b/apps/sim/app/api/workspaces/route.test.ts @@ -47,15 +47,21 @@ vi.mock('@sim/audit', () => ({ vi.mock('@/lib/workspaces/policy', async () => { class WorkspaceCreationCapabilityWithheldError extends Error {} class WorkspaceCreationContextChangedError extends Error {} + class WorkspaceOwnerMissingError extends Error {} return { getWorkspaceCreationPolicy: mockGetWorkspaceCreationPolicy, WorkspaceCreationCapabilityWithheldError, WorkspaceCreationContextChangedError, + WorkspaceOwnerMissingError, } }) -import { WorkspaceCreationCapabilityWithheldError } from '@/lib/workspaces/policy' -import { POST } from '@/app/api/workspaces/route' +import { listWorkspacesForViewer } from '@/lib/workspaces/list' +import { + WorkspaceCreationCapabilityWithheldError, + WorkspaceOwnerMissingError, +} from '@/lib/workspaces/policy' +import { GET, POST } from '@/app/api/workspaces/route' function createRequest() { return createMockRequest('POST', { name: 'New workspace' }) @@ -116,4 +122,27 @@ describe('POST /api/workspaces capability refusal', () => { expect(body.error).toBe('Your organization subscription is inactive.') expect(body.details).toBeUndefined() }) + + /** + * After account deletion the browser's cached session cookie stays valid for + * a few minutes, and the next list load finds no workspaces and tries to + * create the default one for a user who no longer exists. + */ + it('answers a default-workspace insert for a deleted user with 401', async () => { + mockGetSession.mockResolvedValue({ user: { id: 'user-1', name: 'Gone' } }) + vi.mocked(listWorkspacesForViewer).mockResolvedValue({ + workspaces: [], + lastActiveWorkspaceId: null, + pinnedWorkspaceIds: [], + creationPolicy: { canCreate: true, organizationId: null, billedAccountUserId: 'user-1' }, + } as never) + mockCreateWorkspace.mockRejectedValue(new WorkspaceOwnerMissingError('user-1')) + + const response = await GET( + createMockRequest('GET', undefined, undefined, 'http://localhost/api/workspaces?scope=active') + ) + + expect(response.status).toBe(401) + await expect(response.json()).resolves.toEqual({ error: 'Unauthorized' }) + }) }) diff --git a/apps/sim/app/api/workspaces/route.ts b/apps/sim/app/api/workspaces/route.ts index c5b398fa3f1..f2eebe797de 100644 --- a/apps/sim/app/api/workspaces/route.ts +++ b/apps/sim/app/api/workspaces/route.ts @@ -19,6 +19,7 @@ import { getWorkspaceCreationPolicy, WorkspaceCreationCapabilityWithheldError, WorkspaceCreationContextChangedError, + WorkspaceOwnerMissingError, } from '@/lib/workspaces/policy' const logger = createLogger('Workspaces') @@ -87,6 +88,10 @@ export const GET = withRouteHandler(async (request: Request) => { }) return NextResponse.json(refreshedPayload) } + /** A cached session cookie outlived the account it belongs to. */ + if (error instanceof WorkspaceOwnerMissingError) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } throw error } @@ -207,6 +212,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => { { status: 409 } ) } + /** A cached session cookie outlived the account it belongs to. */ + if (error instanceof WorkspaceOwnerMissingError) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } /** * A lock timeout is contention, not a fault: creation serializes on the * organization's mutation locks and now also on `permission_group:`, diff --git a/apps/sim/background/knowledge-connector-sync.test.ts b/apps/sim/background/knowledge-connector-sync.test.ts index 3a34db9c311..4dd9c91e776 100644 --- a/apps/sim/background/knowledge-connector-sync.test.ts +++ b/apps/sim/background/knowledge-connector-sync.test.ts @@ -182,6 +182,20 @@ describe('knowledge connector sync worker', () => { ).toBe('completed') }) + it('keeps a held deletion pass a completed task', () => { + const clean = { + docsAdded: 0, + docsUpdated: 0, + docsDeleted: 0, + docsUnchanged: 90, + docsSkipped: 0, + docsFailed: 0, + processingDispatch: { requested: 0, accepted: 0, failed: 0 }, + } + expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: false })).toBe('completed') + expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: true })).toBe('partial') + }) + it('classifies an isolated processing dispatch failure as partial', () => { expect( classifyConnectorSyncResult({ diff --git a/apps/sim/blocks/blocks/mothership.ts b/apps/sim/blocks/blocks/mothership.ts index e74c7f70f15..2f58bceacf7 100644 --- a/apps/sim/blocks/blocks/mothership.ts +++ b/apps/sim/blocks/blocks/mothership.ts @@ -49,7 +49,7 @@ export const MothershipBlock: BlockConfig = { id: 'conversationId', title: 'Conversation ID', type: 'short-input', - placeholder: 'e.g., user-123, session-abc, customer-456', + placeholder: 'e.g., customer-456 (reuse the same value to continue a thread)', }, { id: 'attachmentFiles', @@ -117,7 +117,8 @@ export const MothershipBlock: BlockConfig = { }, conversationId: { type: 'string', - description: 'Chat ID to continue; generated when omitted', + description: + 'Stable id of the thread to continue; the same value in this workspace continues the same conversation. Generated when omitted', }, files: { type: 'file', @@ -131,7 +132,10 @@ export const MothershipBlock: BlockConfig = { outputs: { content: { type: 'string', description: 'Generated response content' }, model: { type: 'string', description: 'Model used for generation' }, - conversationId: { type: 'string', description: 'Chat ID used for this request' }, + conversationId: { + type: 'string', + description: 'Conversation id for this thread; pass it to another Sim block to continue it', + }, tokens: { type: 'json', description: 'Token usage statistics' }, toolCalls: { type: 'json', description: 'Tool calls made during execution' }, cost: { type: 'json', description: 'Cost of the execution' }, diff --git a/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts b/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts index 6360584cf69..1b0a94d540a 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts @@ -1,4 +1,4 @@ -import { createHash } from 'node:crypto' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' /** * Fixed namespace UUID for fork block-identity derivation. Changing this value @@ -8,30 +8,6 @@ import { createHash } from 'node:crypto' */ const FORK_BLOCK_NAMESPACE = '6f1c0e2a-9b3d-5e47-8a1c-2d4f6b8e0c13' -function uuidToBytes(uuid: string): Buffer { - return Buffer.from(uuid.replace(/-/g, ''), 'hex') -} - -/** - * Deterministic UUIDv5 (SHA-1) of `name` within `namespace`. The same inputs - * always yield the same UUID, which is how fork block identity stays stable. - * - * SHA-1 is mandated by RFC 4122 for UUIDv5 and is used here only for deterministic id derivation, - * never for secrecy or integrity — not a security use of the algorithm. Swapping it would change - * every derived id, breaking webhook URLs and stored block-id references across existing forks - * (see {@link FORK_BLOCK_NAMESPACE}). - */ -function uuidV5(name: string, namespace: string): string { - const hash = createHash('sha1') - hash.update(uuidToBytes(namespace)) // lgtm[js/weak-cryptographic-algorithm] - hash.update(Buffer.from(name, 'utf8')) // lgtm[js/weak-cryptographic-algorithm] - const bytes = hash.digest().subarray(0, 16) - bytes[6] = (bytes[6] & 0x0f) | 0x50 - bytes[8] = (bytes[8] & 0x3f) | 0x80 - const hex = bytes.toString('hex') - return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` -} - /** * Derive the target block id for a source block copied into a target workflow. * diff --git a/apps/sim/executor/handlers/mothership/mothership-handler.test.ts b/apps/sim/executor/handlers/mothership/mothership-handler.test.ts index 0eff6c6a6cc..258e79b3bec 100644 --- a/apps/sim/executor/handlers/mothership/mothership-handler.test.ts +++ b/apps/sim/executor/handlers/mothership/mothership-handler.test.ts @@ -2,6 +2,7 @@ import '@sim/testing/mocks/executor' import { loggerMock, resetEnvMock, setEnv } from '@sim/testing' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' import { BlockType } from '@/executor/constants' import { MothershipBlockHandler } from '@/executor/handlers/mothership/mothership-handler' import type { ExecutionContext, StreamingExecution } from '@/executor/types' @@ -792,7 +793,7 @@ describe('MothershipBlockHandler', () => { messages: [{ role: 'user', content: 'Hello from workflow' }], workspaceId: 'workspace-1', userId: 'user-1', - chatId: 'chat-uuid', + chatId: resolveMothershipConversation('workspace-1', 'chat-uuid').chatId, messageId: 'message-uuid', requestId: 'request-uuid', secretScope: 'all', @@ -876,7 +877,7 @@ describe('MothershipBlockHandler', () => { messages: [{ role: 'user', content: 'Continue this thread' }], workspaceId: 'workspace-1', userId: 'user-1', - chatId: 'existing-chat-id', + chatId: resolveMothershipConversation('workspace-1', 'existing-chat-id').chatId, messageId: 'message-uuid', requestId: 'request-uuid', secretScope: 'all', @@ -887,7 +888,7 @@ describe('MothershipBlockHandler', () => { expect(mockGenerateId).toHaveBeenCalledTimes(2) }) - it('keeps a resolved conversation ID out of logs while forwarding it unchanged', async () => { + it('keeps a resolved conversation ID out of logs and off the wire', async () => { const conversationId = 'chat-plaintext-secret-__var_API_KEY-__sim_secret_API_KEY' mockGenerateId.mockReturnValueOnce('message-uuid').mockReturnValueOnce('request-uuid') fetchMock.mockResolvedValue( @@ -907,7 +908,8 @@ describe('MothershipBlockHandler', () => { const [, options] = fetchMock.mock.calls[0] as [string, RequestInit] const body = JSON.parse(String(options.body)) - expect(body.chatId).toBe(conversationId) + expect(body.chatId).toBe(resolveMothershipConversation('workspace-1', conversationId).chatId) + expect(body.chatId).not.toContain('chat-plaintext-secret') const logged = JSON.stringify(mockMothershipLogger.info.mock.calls) expect(logged).not.toContain('chat-plaintext-secret') @@ -936,7 +938,9 @@ describe('MothershipBlockHandler', () => { const result = await handler.execute(context, block, inputs) const [, options] = fetchMock.mock.calls[0] as [string, RequestInit] - expect(JSON.parse(String(options.body)).chatId).toBe('x') + expect(JSON.parse(String(options.body)).chatId).toBe( + resolveMothershipConversation('workspace-1', 'x').chatId + ) expect(result).toMatchObject({ conversationId: 'x' }) expect(inputs.conversationId).toBe('x') expect(context.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([]) diff --git a/apps/sim/executor/handlers/mothership/mothership-handler.ts b/apps/sim/executor/handlers/mothership/mothership-handler.ts index ba9a5335b2c..f367402ae7d 100644 --- a/apps/sim/executor/handlers/mothership/mothership-handler.ts +++ b/apps/sim/executor/handlers/mothership/mothership-handler.ts @@ -24,6 +24,7 @@ import { import { discoverMcpServerToolsAsExecutor } from '@/lib/internal/mcp/discover-tools' import { assertValidMcpServerToolBindings, MCP_SERVER_ADVANCED_TOOL_TYPE } from '@/lib/mcp/shared' import { resolveMcpToolBinding } from '@/lib/mcp/tool-binding' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' import { areModelSafeWorkspaceFileKeys, MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, @@ -485,7 +486,7 @@ function parseMothershipExecuteStreamLine(line: string): MothershipExecuteStream function formatMothershipBlockOutput( result: MothershipExecuteResult, - fallbackChatId: string + conversationId: string ): NormalizedBlockOutput { const formattedList = (result.toolCalls || []).map((tc: Record) => ({ name: typeof tc.name === 'string' ? tc.name : String(tc.name ?? ''), @@ -503,7 +504,7 @@ function formatMothershipBlockOutput( return { content: result.content || '', model: result.model || 'mothership', - conversationId: result.conversationId || fallbackChatId, + conversationId, tokens: (result.tokens || {}) as NormalizedBlockOutput['tokens'], toolCalls, cost: result.cost as NormalizedBlockOutput['cost'] | undefined, @@ -605,7 +606,7 @@ async function readMothershipExecuteResponse( function createMothershipStreamingExecution( response: Response, - fallbackChatId: string, + conversationId: string, blockId: string, options: { onCancel?: (reason?: unknown) => void @@ -618,7 +619,7 @@ function createMothershipStreamingExecution( throw new Error('Sim execution stream ended without a response body') } - const output = formatMothershipBlockOutput({}, fallbackChatId) + const output = formatMothershipBlockOutput({}, conversationId) let reader: ReadableStreamDefaultReader | undefined let cancelled = false let cleanedUp = false @@ -664,7 +665,7 @@ function createMothershipStreamingExecution( if (event.type === 'final') { await consumeMothershipProvenance(event.data, response, options.registry) sawFinal = true - Object.assign(output, formatMothershipBlockOutput(event.data, fallbackChatId)) + Object.assign(output, formatMothershipBlockOutput(event.data, conversationId)) return } @@ -890,9 +891,10 @@ export class MothershipBlockHandler implements BlockHandler { content: modelInputProjection.value.prompt, }, ] - const providedConversationId = - typeof inputs.conversationId === 'string' ? inputs.conversationId.trim() : '' - const chatId = providedConversationId || generateId() + const { conversationId, chatId } = resolveMothershipConversation( + ctx.workspaceId ?? '', + inputs.conversationId + ) const messageId = generateId() const requestId = generateId() const secretMountPolicy = normalizeSecretMountPolicy({ @@ -1003,15 +1005,20 @@ export class MothershipBlockHandler implements BlockHandler { } if (isContentSelectedForStreaming(ctx, block)) { - const streamingExecution = createMothershipStreamingExecution(response, chatId, block.id, { - onCancel: (reason) => { - if (!abortController.signal.aborted) { - abortController.abort(reason ?? 'mothership_stream_cancelled') - } - }, - onDone: cleanupAbortListeners, - registry: resultRegistry, - }) + const streamingExecution = createMothershipStreamingExecution( + response, + conversationId, + block.id, + { + onCancel: (reason) => { + if (!abortController.signal.aborted) { + abortController.abort(reason ?? 'mothership_stream_cancelled') + } + }, + onDone: cleanupAbortListeners, + registry: resultRegistry, + } + ) streamingExecution.diagnosticResolvedSecretTraceRegistry = resultRegistry if (resultRegistry) ctx.resolvedSecretTraceRegistry = resultRegistry cleanupImmediately = false @@ -1019,7 +1026,7 @@ export class MothershipBlockHandler implements BlockHandler { } const result = await readMothershipExecuteResponse(response, resultRegistry) - const output = formatMothershipBlockOutput(result, chatId) + const output = formatMothershipBlockOutput(result, conversationId) if (resultRegistry) ctx.resolvedSecretTraceRegistry = resultRegistry return output } catch (error) { diff --git a/apps/sim/lib/api/server/routes/internal-json-route.test.ts b/apps/sim/lib/api/server/routes/internal-json-route.test.ts index 32c7dd2a1e5..ace040a22a2 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.test.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.test.ts @@ -492,6 +492,37 @@ describe('defineInternalJsonRoute', () => { }) }) + it('keeps every cookie a finalizer clears on its own header line', async () => { + const handler = defineInternalJsonRoute({ + contract, + auth, + operation, + rateLimit: internalRateLimits.none({ reason: 'Unit test' }), + errorPolicy: internalOrchestrationErrorPolicy, + mapInput: () => undefined, + useCase: { + operation, + async execute() { + return { value: 'ok' } + }, + }, + finalizeResponse: () => ({ + headers: new Headers([ + ['set-cookie', 'session_token=; Max-Age=0; Path=/'], + ['set-cookie', 'session_data=; Max-Age=0; Path=/'], + ]), + }), + }) + + const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route')) + + expect(response.status).toBe(200) + expect(response.headers.getSetCookie()).toEqual([ + 'session_token=; Max-Age=0; Path=/', + 'session_data=; Max-Age=0; Path=/', + ]) + }) + it('selects a declared success status from the application result', async () => { const replayableContract = defineRouteContract({ method: 'POST', diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index ba0d6a5fe0e..ddacfca77da 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -327,6 +327,11 @@ function appendFinalizedHeaders(base: HeadersInit | undefined, additions?: Heade const headers = new Headers(base) if (!additions) return headers new Headers(additions).forEach((value, key) => { + /** A finalizer may clear several cookies at once; each needs its own header line. */ + if (key === 'set-cookie') { + headers.append(key, value) + return + } if (headers.has(key)) { throw new Error(`Internal JSON response finalizer cannot replace header "${key}"`) } diff --git a/apps/sim/lib/core/utils/uuid-v5.test.ts b/apps/sim/lib/core/utils/uuid-v5.test.ts new file mode 100644 index 00000000000..d7bfa33b1af --- /dev/null +++ b/apps/sim/lib/core/utils/uuid-v5.test.ts @@ -0,0 +1,23 @@ +/** + * @vitest-environment node + */ +import { isValidUuid } from '@sim/utils/id' +import { describe, expect, it } from 'vitest' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' + +const DNS_NAMESPACE = '6ba7b810-9dad-11d1-80b4-00c04fd430c8' + +describe('uuidV5', () => { + it('matches the RFC 4122 reference vector', () => { + expect(uuidV5('python.org', DNS_NAMESPACE)).toBe('886313e1-3b8a-5372-9b90-0c9aee199e5d') + }) + + it('is deterministic and namespace-scoped', () => { + const a = uuidV5('customer-456', DNS_NAMESPACE) + expect(uuidV5('customer-456', DNS_NAMESPACE)).toBe(a) + expect(uuidV5('customer-457', DNS_NAMESPACE)).not.toBe(a) + expect(uuidV5('customer-456', '00000000-0000-0000-0000-000000000000')).not.toBe(a) + expect(isValidUuid(a)).toBe(true) + expect(a[14]).toBe('5') + }) +}) diff --git a/apps/sim/lib/core/utils/uuid-v5.ts b/apps/sim/lib/core/utils/uuid-v5.ts new file mode 100644 index 00000000000..9dda65176d4 --- /dev/null +++ b/apps/sim/lib/core/utils/uuid-v5.ts @@ -0,0 +1,26 @@ +import { createHash } from 'node:crypto' + +function uuidToBytes(uuid: string): Buffer { + return Buffer.from(uuid.replace(/-/g, ''), 'hex') +} + +/** + * Deterministic UUIDv5 (SHA-1) of `name` within `namespace`, per RFC 4122 §4.3. + * The same inputs always yield the same UUID, which is how callers derive a + * stable identity from a caller-chosen string. + * + * SHA-1 is mandated by RFC 4122 for UUIDv5 and is used here only for + * deterministic id derivation, never for secrecy or integrity. Every caller + * pins its own namespace constant; changing a namespace re-keys every id that + * caller ever derived. + */ +export function uuidV5(name: string, namespace: string): string { + const hash = createHash('sha1') + hash.update(uuidToBytes(namespace)) // lgtm[js/weak-cryptographic-algorithm] + hash.update(Buffer.from(name, 'utf8')) // lgtm[js/weak-cryptographic-algorithm] + const bytes = hash.digest().subarray(0, 16) + bytes[6] = (bytes[6] & 0x0f) | 0x50 + bytes[8] = (bytes[8] & 0x3f) | 0x80 + const hex = bytes.toString('hex') + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` +} diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts index 61a6fdbe07b..e4b2c551769 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts @@ -2107,6 +2107,25 @@ describe('completeSyncLog', () => { }) }) +describe('isContentPassIncomplete', () => { + it('is true only when the listing has not finished or a source read failed', async () => { + const { isContentPassIncomplete } = await import('@/lib/knowledge/connectors/sync-engine') + const checkpoint = { startedAt: '2026-09-04T00:00:00Z', listedCount: 4 } + for (const complete of [true, false]) { + for (const unsafe of [true, false]) { + for (const contentFailures of [true, false, undefined]) { + expect( + isContentPassIncomplete({ + complete, + checkpoint: { ...checkpoint, unsafe, contentFailures }, + }) + ).toBe(!complete || contentFailures === true) + } + } + } + }) +}) + describe('completeSuccessfulSync', () => { const RESULT = { docsAdded: 1, @@ -2203,6 +2222,48 @@ describe('completeSuccessfulSync', () => { expect((connectorUpdate.nextSyncAt as Date).getTime()).toBeGreaterThan(Date.now() + 50 * 60_000) }) + it('records a held listing as a completed sync whose watermark advances', async () => { + const { completeSuccessfulSync } = await import('@/lib/knowledge/connectors/sync-engine') + queueTableRows(schemaMock.knowledgeBase, [{ id: 'kb-1' }]) + queueTableRows(schemaMock.knowledgeConnector, [{ id: 'c-1' }]) + queueTableRows(schemaMock.document, [{ count: 4 }]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ id: 'log-1' }]) + .mockResolvedValueOnce([{ id: 'c-1' }]) + const holdNotice = 'Source listing is incomplete; unlisted documents were kept.' + + expect( + await completeSuccessfulSync( + 'c-1', + 'kb-1', + 'log-1', + 60, + { ...RESULT, docsFailed: 0 }, + holdNotice, + { + complete: true, + checkpoint: { + unsafe: true, + contentFailures: false, + startedAt: '2026-09-04T00:00:00Z', + listedCount: 4, + }, + } + ) + ).toBe(true) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ status: 'completed', docsFailed: 0, listedCount: 4 }) + ) + const connectorUpdate = dbChainMockFns.set.mock.calls.find( + (call) => (call[0] as Record | undefined)?.status === 'active' + )?.[0] as Record + expect(connectorUpdate.lastSyncAt).toEqual(new Date('2026-09-04T00:00:00Z')) + expect(connectorUpdate.lastSyncError).toBe(holdNotice) + expect(connectorUpdate.listingCheckpoint).toBeNull() + expect((connectorUpdate.nextSyncAt as Date).getTime()).toBeGreaterThan(Date.now() + 50 * 60_000) + }) + it('does not publish connector state when the guarded log close is refused', async () => { const { completeSuccessfulSync } = await import('@/lib/knowledge/connectors/sync-engine') diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 98348acc60e..d1e9ef65b85 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -289,6 +289,32 @@ class SyncCompletionOwnershipLost extends Error { } } +/** What a finished content pass reports to the sync-log and connector close. */ +export interface ContentPassOutcome { + complete: boolean + checkpoint: { + unsafe: boolean + contentFailures?: boolean + startedAt: string + listedCount: number + incrementalSince?: string | null + } +} + +/** + * A content pass is incomplete when the listing has not reached the end of the + * source (the generation resumes on the next run) or a source read failed (the + * next pass replays it). `checkpoint.unsafe` is deliberately not part of this: + * it means "do not infer deletions from this listing" and is honored by the + * deletion hold in `reconcileCompletedListing`. A held pass is still a + * completed sync whose watermark advances. + */ +export function isContentPassIncomplete( + contentPass: Pick +): boolean { + return !contentPass.complete || contentPass.checkpoint.contentFailures === true +} + /** * Atomically publishes the completed log and connector terminal state. * @@ -303,16 +329,7 @@ export async function completeSuccessfulSync( syncIntervalMinutes: number, result: SyncResult, reconciliationHoldNotice: string | null, - contentPass?: { - complete: boolean - checkpoint: { - unsafe: boolean - contentFailures?: boolean - startedAt: string - listedCount: number - incrementalSince?: string | null - } - } + contentPass?: ContentPassOutcome ): Promise { try { return await db.transaction(async (tx) => { @@ -361,13 +378,7 @@ export async function completeSuccessfulSync( const [closedLog] = await tx .update(knowledgeConnectorSyncLog) .set({ - status: - contentPass && - (!contentPass.complete || - contentPass.checkpoint.unsafe || - contentPass.checkpoint.contentFailures) - ? 'partial' - : 'completed', + status: contentPass && isContentPassIncomplete(contentPass) ? 'partial' : 'completed', completedAt: now, listedCount: contentPass?.complete ? contentPass.checkpoint.incrementalSince @@ -398,19 +409,12 @@ export async function completeSuccessfulSync( actualDocCount, contentPass && !contentPass.complete ? now : calculateNextSyncTime(syncIntervalMinutes), reconciliationHoldNotice, - result.docsFailed === 0 && - (!contentPass || - (contentPass.complete && - !contentPass.checkpoint.unsafe && - !contentPass.checkpoint.contentFailures)) + result.docsFailed === 0 && (!contentPass || !isContentPassIncomplete(contentPass)) ), /** Restored above under this same lock, or hidden by the admin pass before the ACLs it wrote. */ accessRewritePending: false, ...(contentPass?.complete ? { listingCheckpoint: null } : {}), - ...(contentPass?.complete && - !contentPass.checkpoint.unsafe && - !contentPass.checkpoint.contentFailures && - result.docsFailed === 0 + ...(contentPass && !isContentPassIncomplete(contentPass) && result.docsFailed === 0 ? { lastSyncAt: new Date(contentPass.checkpoint.startedAt) } : {}), }) @@ -1119,10 +1123,7 @@ export async function executeSync( : undefined, }) - result.listingIncomplete = - !contentPass.complete || - contentPass.checkpoint.unsafe || - contentPass.checkpoint.contentFailures + result.listingIncomplete = isContentPassIncomplete(contentPass) const reconciliationHoldNotice = contentPass.holdNotice const directoryError = await directoryRefreshed if (directoryError) throw directoryError diff --git a/apps/sim/lib/mothership/conversation-id.test.ts b/apps/sim/lib/mothership/conversation-id.test.ts new file mode 100644 index 00000000000..24fa3f79eda --- /dev/null +++ b/apps/sim/lib/mothership/conversation-id.test.ts @@ -0,0 +1,60 @@ +/** + * @vitest-environment node + */ +import { isValidUuid } from '@sim/utils/id' +import { describe, expect, it } from 'vitest' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' + +const WORKSPACE_A = '11111111-1111-4111-8111-111111111111' +const WORKSPACE_B = '22222222-2222-4222-8222-222222222222' + +describe('resolveMothershipConversation', () => { + it('mints a fresh token when no conversation id is given', () => { + const first = resolveMothershipConversation(WORKSPACE_A, undefined) + const second = resolveMothershipConversation(WORKSPACE_A, ' ') + expect(isValidUuid(first.conversationId)).toBe(true) + expect(isValidUuid(first.chatId)).toBe(true) + expect(first.chatId).not.toBe(first.conversationId) + expect(first.chatId).not.toBe(second.chatId) + }) + + it('derives a stable chat id scoped to the workspace and exposes the given id', () => { + const resolved = resolveMothershipConversation(WORKSPACE_A, ' customer-456 ') + expect(resolved.conversationId).toBe('customer-456') + expect(isValidUuid(resolved.chatId)).toBe(true) + expect(resolveMothershipConversation(WORKSPACE_A, 'customer-456').chatId).toBe(resolved.chatId) + expect(resolveMothershipConversation(WORKSPACE_A, 'customer-457').chatId).not.toBe( + resolved.chatId + ) + expect(resolveMothershipConversation(WORKSPACE_B, 'customer-456').chatId).not.toBe( + resolved.chatId + ) + }) + + it('derives UUID-shaped ids too, so a block can never reach a chat it did not derive', () => { + const pasted = '3b2f0d4e-8a6c-4f1b-9e2d-5c7a1b3d9f00' + const resolved = resolveMothershipConversation(WORKSPACE_A, pasted) + expect(resolved.conversationId).toBe(pasted) + expect(resolved.chatId).not.toBe(pasted) + expect(isValidUuid(resolved.chatId)).toBe(true) + }) + + it('continues the same thread when the exposed id is chained into another block', () => { + const first = resolveMothershipConversation(WORKSPACE_A, undefined) + const chained = resolveMothershipConversation(WORKSPACE_A, first.conversationId) + expect(chained.chatId).toBe(first.chatId) + }) + + it('never forwards the literal string', () => { + const secretish = 'chat-plaintext-secret-__var_API_KEY-__sim_secret_API_KEY' + const { chatId } = resolveMothershipConversation(WORKSPACE_A, secretish) + expect(chatId).not.toContain('secret') + expect(chatId).not.toContain('__') + }) + + it('refuses to resolve without a workspace', () => { + expect(() => resolveMothershipConversation('', 'customer-456')).toThrow( + 'Workspace context is required' + ) + }) +}) diff --git a/apps/sim/lib/mothership/conversation-id.ts b/apps/sim/lib/mothership/conversation-id.ts new file mode 100644 index 00000000000..47224e109bf --- /dev/null +++ b/apps/sim/lib/mothership/conversation-id.ts @@ -0,0 +1,45 @@ +import { generateId } from '@sim/utils/id' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' + +/** + * Namespace for chat ids derived from block conversation ids. Changing it + * re-keys every derived conversation, so every running block thread would + * lose its history. It must never change. + */ +const MOTHERSHIP_CONVERSATION_NAMESPACE = '2b7c5e0a-4f61-5d3e-9a8b-7c1d0e2f3a45' + +export interface ResolvedMothershipConversation { + /** The id the block exposes; feeding it to another Sim block continues the thread. */ + conversationId: string + /** The chat id sent over the wire, derived from the workspace and the conversation id. */ + chatId: string +} + +/** + * Resolves the chat id a Sim block conversation runs under. + * + * Builders may choose any stable string such as `customer-456` so a workflow + * continues one thread per customer, and chat ids are UUIDs everywhere + * downstream (`copilot_chats.id`, `workspace_files.chat_id`, and the copilot + * service's own conversation store). Every conversation id, UUID-shaped or not, + * is mapped to a UUID derived from the workspace and the id: the same value + * keeps the same thread across runs, two workspaces choosing the same value + * never share a conversation, a block can never reach a chat it did not derive + * (the copilot store has no ownership check of its own), and the literal value + * (which may carry a resolved secret) never leaves the executor. An omitted id + * mints a fresh token so the exposed id still continues the thread when chained. + */ +export function resolveMothershipConversation( + workspaceId: string, + conversationId: unknown +): ResolvedMothershipConversation { + if (!workspaceId) { + throw new Error('Workspace context is required to resolve a Sim conversation') + } + const provided = typeof conversationId === 'string' ? conversationId.trim() : '' + const token = provided || generateId() + return { + conversationId: token, + chatId: uuidV5(`${workspaceId}:${token}`, MOTHERSHIP_CONVERSATION_NAMESPACE), + } +} diff --git a/apps/sim/lib/workspaces/create.test.ts b/apps/sim/lib/workspaces/create.test.ts index 771740f8fca..af282f9c778 100644 --- a/apps/sim/lib/workspaces/create.test.ts +++ b/apps/sim/lib/workspaces/create.test.ts @@ -47,7 +47,7 @@ import { createDefaultPersonalWorkspaceInTransaction, createWorkspace, } from '@/lib/workspaces/create' -import { WORKSPACE_MODE } from '@/lib/workspaces/policy' +import { WORKSPACE_MODE, WorkspaceOwnerMissingError } from '@/lib/workspaces/policy' const params = { userId: 'creator-1', @@ -127,6 +127,34 @@ describe('createWorkspace capability-gate placement', () => { }) }) + /** + * A cached session cookie can outlive the user row by a few minutes. The + * insert then fails on a `workspace` -> `user` foreign key, which the caller + * must be able to tell apart from a fault so it answers 401, not 500. + */ + it('reports a missing owner as a typed error instead of a fault', async () => { + mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1') + dbChainMockFns.transaction.mockRejectedValue( + Object.assign(new Error('insert or update on table "workspace" violates foreign key'), { + code: '23503', + constraint_name: 'workspace_billed_account_user_id_user_id_fk', + }) + ) + + await expect(createWorkspace(params)).rejects.toBeInstanceOf(WorkspaceOwnerMissingError) + }) + + it('rethrows other foreign key violations untouched', async () => { + mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1') + const failure = Object.assign(new Error('violates foreign key'), { + code: '23503', + constraint_name: 'workspace_organization_id_organization_id_fk', + }) + dbChainMockFns.transaction.mockRejectedValue(failure) + + await expect(createWorkspace(params)).rejects.toBe(failure) + }) + /** * The preflight policy resolved this value microseconds earlier in the same * request, and React's `cache()` memo does not span the two calls, so a diff --git a/apps/sim/lib/workspaces/create.ts b/apps/sim/lib/workspaces/create.ts index 44d4e8e77b3..8e5d8f46c0b 100644 --- a/apps/sim/lib/workspaces/create.ts +++ b/apps/sim/lib/workspaces/create.ts @@ -1,6 +1,7 @@ import { db } from '@sim/db' import { permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { PlatformEvents } from '@/lib/core/telemetry' import type { DbOrTx } from '@/lib/db/types' @@ -12,8 +13,15 @@ import { resolveGoverningPermissionGroupOrganization, resolveInviteFlags, WORKSPACE_MODE, + WorkspaceOwnerMissingError, } from '@/lib/workspaces/policy' +/** Foreign keys from `workspace` to `user`; a violation means the acting user's row is gone. */ +const WORKSPACE_USER_FK_CONSTRAINTS = new Set([ + 'workspace_owner_id_user_id_fk', + 'workspace_billed_account_user_id_user_id_fk', +]) + const logger = createLogger('WorkspaceCreate') export interface CreateWorkspaceParams { @@ -208,6 +216,13 @@ export async function createWorkspace(params: CreateWorkspaceParams) { createWorkspaceInTransaction(tx, { ...params, governingPermissionGroupOrganizationId }) ) } catch (error) { + if ( + getPostgresErrorCode(error) === '23503' && + WORKSPACE_USER_FK_CONSTRAINTS.has(getPostgresConstraintName(error) ?? '') + ) { + logger.warn('Workspace creation raced account deletion', { userId: params.userId }) + throw new WorkspaceOwnerMissingError(params.userId) + } logger.error('Failed to create workspace', { userId: params.userId, error }) throw error } diff --git a/apps/sim/lib/workspaces/policy.ts b/apps/sim/lib/workspaces/policy.ts index 3b511b15945..42c2e336eb1 100644 --- a/apps/sim/lib/workspaces/policy.ts +++ b/apps/sim/lib/workspaces/policy.ts @@ -111,6 +111,18 @@ export interface WorkspaceCreationPolicy { blockedReasonCode?: 'organization-subscription-inactive' | 'permission-group-denied' } +/** + * The acting user's row is gone, so no workspace can reference it. Reached + * when a request still carrying a cached session cookie arrives after the + * account was deleted; the caller should answer as unauthenticated. + */ +export class WorkspaceOwnerMissingError extends Error { + constructor(userId: string) { + super(`User ${userId} no longer exists`) + this.name = 'WorkspaceOwnerMissingError' + } +} + export class WorkspaceCreationContextChangedError extends Error { constructor(message = 'Workspace creation context changed before the workspace was inserted') { super(message)