diff --git a/apps/sim/app/api/knowledge/member-connectors/route.ts b/apps/sim/app/api/knowledge/member-connectors/route.ts index 4467c3ec35e..1c9db2a0230 100644 --- a/apps/sim/app/api/knowledge/member-connectors/route.ts +++ b/apps/sim/app/api/knowledge/member-connectors/route.ts @@ -13,7 +13,7 @@ export const GET = defineInternalJsonRoute({ auth: internalSessionAuth, operation: knowledgeOperations.listWorkspaceMemberConnectors, rateLimit: internalRateLimits.none({ reason: 'One small read per visit to the Search tab' }), - errorPolicy: internalKnowledgeErrorPolicies.connectors, + errorPolicy: internalKnowledgeErrorPolicies.memberConnectors, mapInput: ({ query }) => ({ workspaceId: query.workspaceId }), useCase: listWorkspaceMemberConnectors, present: ({ connectors }) => ({ success: true as const, data: connectors }), diff --git a/apps/sim/app/api/v1/knowledge/[id]/documents/[documentId]/route.ts b/apps/sim/app/api/v1/knowledge/[id]/documents/[documentId]/route.ts index 7ad5558439e..3e2c415daf4 100644 --- a/apps/sim/app/api/v1/knowledge/[id]/documents/[documentId]/route.ts +++ b/apps/sim/app/api/v1/knowledge/[id]/documents/[documentId]/route.ts @@ -113,11 +113,12 @@ export const DELETE = withRouteHandler( ) if (result instanceof NextResponse) return result - const doc = await getKnowledgeDocument( - knowledgeBaseId, - documentId, - await resolveV1KnowledgeAccessScope(userId, rateLimit, parsed.data.query.workspaceId) + const access = await resolveV1KnowledgeAccessScope( + userId, + rateLimit, + parsed.data.query.workspaceId ) + const doc = await getKnowledgeDocument(knowledgeBaseId, documentId, access) if (!doc) { return NextResponse.json({ error: 'Document not found' }, { status: 404 }) @@ -130,6 +131,7 @@ export const DELETE = withRouteHandler( workspaceId: parsed.data.query.workspaceId, }, document: { id: documentId, filename: doc.filename }, + access, userId, source: 'api', requestId, diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 014e0058998..a27ce2f1021 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -543,23 +543,28 @@ export function Home({ chatId, userName, userId }: HomeProps) { ] ) + /** An emptied search box returns to the sources; a send in any other mode has no search to clear. */ + const clearSearch = useCallback(() => { + if (searchQueryValue !== null) setSearchQuery('') + }, [searchQueryValue, setSearchQuery]) + /** * A queued message re-enters the composer in the mode it was written in: an * Assistant question edits as an Assistant question, and never as a Search, * which submits nothing and would leave the edit stranded. + * + * The live query goes with it. A restored mode is never Search, and a query + * left in the URL pulls the composer straight back to Search, which is the + * one mode the edit cannot be sent from. */ const restoreQueuedMode = useCallback( (requestMode: QueuedMessage['requestMode']) => { void setComposerMode(requestMode === 'ask' ? 'assistant' : 'build') + clearSearch() }, - [setComposerMode] + [clearSearch, setComposerMode] ) - /** An emptied search box returns to the sources; a send in any other mode has no search to clear. */ - const clearSearch = useCallback(() => { - if (searchQueryValue !== null) setSearchQuery('') - }, [searchQueryValue, setSearchQuery]) - /** * Summarize or Answer on a result: switch to Assistant and hand the question * to it. The submit reads the mode from this render, so it is sent as an diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.test.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.test.ts new file mode 100644 index 00000000000..27bc575e62c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.test.ts @@ -0,0 +1,47 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/hooks/queries/credential-groups', () => ({ useCredentialGroups: vi.fn() })) + +import { connectorMemberGroupProvider } from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options' +import { CONNECTOR_META_REGISTRY } from '@/connectors/registry' +import type { ConnectorMeta } from '@/connectors/types' + +const permissionScoped = Object.values(CONNECTOR_META_REGISTRY).filter( + (meta) => meta.permissionScopedListing !== undefined +) + +describe('connectorMemberGroupProvider', () => { + it.each(permissionScoped.map((meta) => [meta.id, meta] as const))( + 'resolves %s, so its Access control renders', + (_id, meta) => { + expect(connectorMemberGroupProvider(meta)).not.toBeNull() + } + ) + + /** + * Slack authorizes through the workspace's own app rather than Sim's OAuth + * client. Resolving only the providers Sim owns a client for left it with no + * way into per-member access and no way back out of it. + */ + it('resolves Slack, which no standard OAuth client backs', () => { + const slack = CONNECTOR_META_REGISTRY.slack + expect(slack.permissionScopedListing).toBeDefined() + expect(connectorMemberGroupProvider(slack)).toBe('slack') + }) + + it('resolves nothing for a connector no Credential Group collects accounts for', () => { + const sftp = CONNECTOR_META_REGISTRY.sftp + expect(connectorMemberGroupProvider(sftp)).toBeNull() + }) + + it('resolves nothing for a connector whose listing is not permission scoped', () => { + const withoutScopedListing = { + ...CONNECTOR_META_REGISTRY.slack, + permissionScopedListing: undefined, + } as ConnectorMeta + expect(connectorMemberGroupProvider(withoutScopedListing)).toBeNull() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.ts index 6806f4c8af4..e7a3a36f73f 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.ts +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.ts @@ -3,10 +3,10 @@ import { useMemo } from 'react' import type { ComboboxOption } from '@sim/emcn' import { - type CredentialGroupStandardOAuthProvider, + type CredentialGroupProvider, + findCredentialGroupProviderByProviderId, getCredentialGroupProviderId, - getCredentialGroupStandardOAuthProviderFromProviderId, - isCredentialGroupProvider, + isSelectableCredentialGroupOption, } from '@/lib/credential-groups/providers' import type { ConnectorMeta } from '@/connectors/types' import { useCredentialGroups } from '@/hooks/queries/credential-groups' @@ -30,16 +30,18 @@ export function decodeConnectorMemberGroupOption( } } -/** The credential-group provider that collects accounts for this connector, if any. */ -function connectorMemberGroupProvider( +/** + * The credential-group provider that collects accounts for this connector, if + * any. Every provider counts, not only those Sim authorizes with its own OAuth + * client: Slack collects accounts through the workspace's own app, and asking + * only about the standard ones leaves a Slack connector with no Access control + * at all. + */ +export function connectorMemberGroupProvider( connectorConfig: ConnectorMeta -): CredentialGroupStandardOAuthProvider | null { +): CredentialGroupProvider | null { if (connectorConfig.auth.mode !== 'oauth' || !connectorConfig.permissionScopedListing) return null - try { - return getCredentialGroupStandardOAuthProviderFromProviderId(connectorConfig.auth.provider) - } catch { - return null - } + return findCredentialGroupProviderByProviderId(connectorConfig.auth.provider) } /** The config fields a per-member connector hides: its listing caps, which the server clears. */ @@ -94,9 +96,8 @@ export function useConnectorMemberGroupOptions({ for (const group of settings.credentialGroups) { if (group.status !== 'active') continue for (const option of group.options) { - if (option.status !== 'active') continue - if (!isCredentialGroupProvider(option.provider)) continue - if (getCredentialGroupProviderId(option.provider) !== providerId) continue + /** The server's own rule, so the picker never offers an option it would refuse. */ + if (!isSelectableCredentialGroupOption(option, providerId)) continue entries.push({ label: `${group.name} · ${option.label}`, value: encodeConnectorMemberGroupOption(group.id, option.id), diff --git a/apps/sim/lib/credential-groups/application/create-invite-link.ts b/apps/sim/lib/credential-groups/application/create-invite-link.ts index 9290ba46e59..2ba3a0d4edb 100644 --- a/apps/sim/lib/credential-groups/application/create-invite-link.ts +++ b/apps/sim/lib/credential-groups/application/create-invite-link.ts @@ -10,8 +10,8 @@ import { } from '@/lib/credential-groups/application/context' import { credentialGroupOperations } from '@/lib/credential-groups/application/operations' import { - CredentialGroupEnrollmentError, createCredentialGroupInvitationLink, + rethrowEnrollmentErrorAsOrchestrationError, } from '@/lib/credential-groups/enrollments' export interface CreateCredentialGroupInviteLinkInput { @@ -45,13 +45,7 @@ export const createCredentialGroupInviteLink = defineAuthorizedWorkspaceUseCase( email ) } catch (error) { - if (error instanceof CredentialGroupEnrollmentError) { - throw new OrchestrationError( - error.status === 404 ? 'not_found' : error.status === 409 ? 'conflict' : 'internal', - error.message - ) - } - throw error + rethrowEnrollmentErrorAsOrchestrationError(error) } }, projectAudit: ({ context, result }) => ({ diff --git a/apps/sim/lib/credential-groups/application/manage-enrollments.ts b/apps/sim/lib/credential-groups/application/manage-enrollments.ts index 970a98d74b3..ba14303a8de 100644 --- a/apps/sim/lib/credential-groups/application/manage-enrollments.ts +++ b/apps/sim/lib/credential-groups/application/manage-enrollments.ts @@ -8,11 +8,11 @@ import { import { credentialGroupOperations } from '@/lib/credential-groups/application/operations' import { validateCredentialGroupInvitationEmails } from '@/lib/credential-groups/application/validation' import { - CredentialGroupEnrollmentError, deleteCredentialGroupEnrollment, inviteCredentialGroupEnrollments, loadCredentialGroupInviterIdentity, resendCredentialGroupEnrollment, + rethrowEnrollmentErrorAsOrchestrationError, } from '@/lib/credential-groups/enrollments' import { mcpService } from '@/lib/mcp/service' @@ -21,14 +21,6 @@ interface CredentialGroupEnrollmentSettingsInput { credentialGroupId: string } -function normalizeEnrollmentError(error: unknown): never { - if (error instanceof CredentialGroupEnrollmentError) { - if (error.status === 404) throw new OrchestrationError('not_found', error.message) - if (error.status === 409) throw new OrchestrationError('conflict', error.message) - } - throw error -} - async function requireInviterIdentity(userId: string): Promise { const inviter = await loadCredentialGroupInviterIdentity(userId) const inviterName = inviter?.name?.trim() || inviter?.email @@ -61,7 +53,7 @@ export const inviteCredentialGroupEnrollmentsSettings = defineAuthorizedWorkspac { emails } ) } catch (error) { - normalizeEnrollmentError(error) + rethrowEnrollmentErrorAsOrchestrationError(error) } }, projectAudit: ({ context, result }) => ({ @@ -97,7 +89,7 @@ export const resendCredentialGroupEnrollmentSettings = defineAuthorizedWorkspace ) return { credentialGroupEnrollment } } catch (error) { - normalizeEnrollmentError(error) + rethrowEnrollmentErrorAsOrchestrationError(error) } }, projectAudit: ({ context, result }) => ({ @@ -129,7 +121,7 @@ export const deleteCredentialGroupEnrollmentSettings = defineAuthorizedWorkspace input.enrollmentId ) } catch (error) { - normalizeEnrollmentError(error) + rethrowEnrollmentErrorAsOrchestrationError(error) } }, projectAudit: ({ context, result }) => ({ diff --git a/apps/sim/lib/credential-groups/application/send-invite.ts b/apps/sim/lib/credential-groups/application/send-invite.ts index 3e970b81675..9d66bf940c8 100644 --- a/apps/sim/lib/credential-groups/application/send-invite.ts +++ b/apps/sim/lib/credential-groups/application/send-invite.ts @@ -13,9 +13,9 @@ import { } from '@/lib/credential-groups/application/context' import { credentialGroupOperations } from '@/lib/credential-groups/application/operations' import { - CredentialGroupEnrollmentError, inviteCredentialGroupEnrollment, loadCredentialGroupInviterIdentity, + rethrowEnrollmentErrorAsOrchestrationError, } from '@/lib/credential-groups/enrollments' export interface SendCredentialGroupInviteInput { @@ -57,13 +57,7 @@ export const sendCredentialGroupInvite = defineAuthorizedWorkspaceUseCase({ ) return { enrollment } } catch (error) { - if (error instanceof CredentialGroupEnrollmentError) { - throw new OrchestrationError( - error.status === 404 ? 'not_found' : error.status === 409 ? 'conflict' : 'internal', - error.message - ) - } - throw error + rethrowEnrollmentErrorAsOrchestrationError(error) } }, projectAudit: ({ input, context, result }) => ({ diff --git a/apps/sim/lib/credential-groups/enrollments.ts b/apps/sim/lib/credential-groups/enrollments.ts index 7ac30a2c80a..b3b5db3dbd2 100644 --- a/apps/sim/lib/credential-groups/enrollments.ts +++ b/apps/sim/lib/credential-groups/enrollments.ts @@ -16,6 +16,7 @@ import { and, asc, count, desc, eq, inArray, isNull, lt, or, sql } from 'drizzle import { renderCredentialGroupInvitationEmail } from '@/components/emails/credential-groups/render' import { getCredentialGroupInvitationSubject } from '@/components/emails/subjects' import { getWorkspaceOwnerSubscriptionAccess } from '@/lib/billing/core/workspace-access' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { getBaseUrl } from '@/lib/core/utils/urls' import { isCredentialGroupsAvailable } from '@/lib/credential-groups/availability' import type { ManagedMcpConnectorId } from '@/lib/credential-groups/managed-mcp-connectors' @@ -182,6 +183,36 @@ export class CredentialGroupEnrollmentError extends Error { } } +/** + * Rethrows an enrollment failure as the orchestration error its status already + * implies. The mapping belongs to the error rather than to each caller: every + * surface that issues an invitation reports a missing group, a disabled one, or + * one collecting no accounts the same way, and a status added here reaches all + * of them at once. Anything that is not an enrollment failure passes through + * untouched. + */ +export function rethrowEnrollmentErrorAsOrchestrationError(error: unknown): never { + if (error instanceof CredentialGroupEnrollmentError) { + /** + * Every status the error can carry is mapped, so none falls through to a + * bare rethrow and loses its message to a generic internal failure. A 502 + * is upstream rather than the caller's fault, but saying which upstream + * step failed is still the useful half of the answer. + */ + switch (error.status) { + case 400: + throw new OrchestrationError('validation', error.message) + case 404: + throw new OrchestrationError('not_found', error.message) + case 409: + throw new OrchestrationError('conflict', error.message) + case 502: + throw new OrchestrationError('internal', error.message) + } + } + throw error +} + interface CredentialGroupEnrollmentCursor { id: string invitedAt: Date diff --git a/apps/sim/lib/credential-groups/providers.ts b/apps/sim/lib/credential-groups/providers.ts index 09638d94ef2..6ae33b618a4 100644 --- a/apps/sim/lib/credential-groups/providers.ts +++ b/apps/sim/lib/credential-groups/providers.ts @@ -262,6 +262,39 @@ export function getCredentialGroupProviderId(provider: CredentialGroupProvider): return getCredentialGroupProviderService(provider).providerId } +/** + * The credential-group provider collecting accounts for a service, or null when + * none does. The throwing form below stays for callers that have already + * established the provider exists; this one is for asking the question. + */ +export function findCredentialGroupProviderByProviderId( + providerId: string +): CredentialGroupProvider | null { + return ( + CREDENTIAL_GROUP_PROVIDER_IDS.find( + (candidate) => getCredentialGroupProviderId(candidate) === providerId + ) ?? null + ) +} + +/** + * Whether an option is one a connector may actually sync through: live, and — + * for a provider configured per group, such as Slack through the workspace's + * own app — configured. Shared so the settings UI offers exactly the options + * the server would go on to pick, rather than one it will refuse. + */ +export function isSelectableCredentialGroupOption( + option: { provider: string; status: string; configurationStatus: string }, + providerId: string +): boolean { + return ( + option.status === 'active' && + option.configurationStatus === 'ready' && + isCredentialGroupProvider(option.provider) && + getCredentialGroupProviderId(option.provider) === providerId + ) +} + export function getCredentialGroupProviderFromProviderId( providerId: string ): CredentialGroupProvider { diff --git a/apps/sim/lib/knowledge/api/route-policies.ts b/apps/sim/lib/knowledge/api/route-policies.ts index 291585938dd..c2ee226d116 100644 --- a/apps/sim/lib/knowledge/api/route-policies.ts +++ b/apps/sim/lib/knowledge/api/route-policies.ts @@ -84,6 +84,8 @@ export const internalKnowledgeErrorPolicies = { */ bulkMove: internalKnowledgeErrorPolicy('Failed to move knowledge bases'), bulkDelete: internalKnowledgeErrorPolicy('Failed to delete knowledge bases'), + /** Workspace-scoped, so unconcealed for the same reason as the bulk routes above. */ + memberConnectors: internalKnowledgeErrorPolicy('Failed to fetch member connectors'), default: internalKnowledgeErrorPolicy('Internal server error'), documents: concealKnowledgeBase( internalKnowledgeErrorPolicy('Failed to process knowledge document request') diff --git a/apps/sim/lib/knowledge/connectors/member-provisioning.ts b/apps/sim/lib/knowledge/connectors/member-provisioning.ts index 3b9fa1ec37e..c7e28da8504 100644 --- a/apps/sim/lib/knowledge/connectors/member-provisioning.ts +++ b/apps/sim/lib/knowledge/connectors/member-provisioning.ts @@ -15,13 +15,13 @@ import { CredentialGroupEnrollmentError, createCredentialGroupInvitationLink, inviteCredentialGroupEnrollment, + rethrowEnrollmentErrorAsOrchestrationError, } from '@/lib/credential-groups/enrollments' import { type CredentialGroupProvider, - getCredentialGroupProviderFromProviderId, - getCredentialGroupProviderId, - isCredentialGroupProvider, + findCredentialGroupProviderByProviderId, isCredentialGroupStandardOAuthProvider, + isSelectableCredentialGroupOption, } from '@/lib/credential-groups/providers' import { createCredentialGroup, listCredentialGroups } from '@/lib/credential-groups/service' import { isKnowledgeMemberAccessAvailable } from '@/lib/knowledge/access/availability' @@ -115,10 +115,9 @@ export async function provisionKnowledgeConnectorMembersBinding(input: { throw new OrchestrationError('validation', 'Only an OAuth connector can sync per member') } const providerId = connectorMeta.auth.provider - let provider: CredentialGroupProvider - try { - provider = getCredentialGroupProviderFromProviderId(providerId) - } catch { + const provider: CredentialGroupProvider | null = + findCredentialGroupProviderByProviderId(providerId) + if (!provider) { throw new OrchestrationError( 'validation', `${connectorMeta.name} accounts cannot be collected through a Credential Group yet` @@ -130,9 +129,7 @@ export async function provisionKnowledgeConnectorMembersBinding(input: { for (const group of groups) { if (group.status !== 'active') continue for (const option of group.options) { - if (option.status !== 'active' || option.configurationStatus !== 'ready') continue - if (!isCredentialGroupProvider(option.provider)) continue - if (getCredentialGroupProviderId(option.provider) !== providerId) continue + if (!isSelectableCredentialGroupOption(option, providerId)) continue candidates.push({ credentialGroupId: group.id, credentialGroupOptionId: option.id }) } } @@ -393,7 +390,12 @@ export async function createViewerConnectorEnrollmentLink(input: { ) { throw revoked } - throw error + /** + * Everything else the issue refuses is a state of the group the person can + * be told about — it is gone, disabled, or collects no accounts yet — so it + * carries its own status out rather than surfacing as an internal error. + */ + rethrowEnrollmentErrorAsOrchestrationError(error) } } diff --git a/apps/sim/lib/knowledge/orchestration/documents.test.ts b/apps/sim/lib/knowledge/orchestration/documents.test.ts index 2da863155df..ecf80f1dace 100644 --- a/apps/sim/lib/knowledge/orchestration/documents.test.ts +++ b/apps/sim/lib/knowledge/orchestration/documents.test.ts @@ -8,7 +8,7 @@ const { mockCaptureServerEvent, mockCreateDocumentRecords, mockCreateSingleDocument, - mockDeleteDocument, + mockDeleteKnowledgeDocumentInKnowledgeBase, mockGetDocumentByUploadId, mockMarkDocumentAsFailedTimeout, mockProcessDocumentAsync, @@ -21,7 +21,7 @@ const { mockCaptureServerEvent: vi.fn(), mockCreateDocumentRecords: vi.fn(), mockCreateSingleDocument: vi.fn(), - mockDeleteDocument: vi.fn(), + mockDeleteKnowledgeDocumentInKnowledgeBase: vi.fn(), mockGetDocumentByUploadId: vi.fn(), mockMarkDocumentAsFailedTimeout: vi.fn(), mockProcessDocumentAsync: vi.fn(), @@ -47,7 +47,7 @@ vi.mock('@/lib/core/telemetry', () => ({ vi.mock('@/lib/knowledge/documents/service', () => ({ createDocumentRecords: mockCreateDocumentRecords, createSingleDocument: mockCreateSingleDocument, - deleteDocument: mockDeleteDocument, + deleteKnowledgeDocumentInKnowledgeBase: mockDeleteKnowledgeDocumentInKnowledgeBase, getDocumentByUploadId: mockGetDocumentByUploadId, markDocumentAsFailedTimeout: mockMarkDocumentAsFailedTimeout, processDocumentAsync: mockProcessDocumentAsync, @@ -75,6 +75,7 @@ const FILE = { mimeType: 'application/pdf', } const ACTOR = { userId: 'user-1', source: 'agent' as const, requestId: 'req-1' } +const ACCESS = { kind: 'user' as const, userId: 'user-1', tokens: ['ws', 'pub'] as const } /** * Lets the fire-and-forget dispatch settle. Both upload paths queue indexing @@ -403,18 +404,24 @@ describe('performUpdateKnowledgeDocument', () => { describe('performDeleteKnowledgeDocument', () => { beforeEach(() => { vi.clearAllMocks() - mockDeleteDocument.mockResolvedValue({ success: true, message: 'ok' }) + mockDeleteKnowledgeDocumentInKnowledgeBase.mockResolvedValue(undefined) }) - it('audits the deletion against the acting user', async () => { + it('deletes within the knowledge base under the caller’s access, and audits it', async () => { const outcome = await performDeleteKnowledgeDocument({ ...ACTOR, knowledgeBase: KB, document: { id: 'doc-1', filename: 'report.pdf', fileSize: 10, mimeType: 'application/pdf' }, + access: ACCESS, }) expect(outcome).toMatchObject({ success: true }) - expect(mockDeleteDocument).toHaveBeenCalledWith('doc-1', 'req-1') + expect(mockDeleteKnowledgeDocumentInKnowledgeBase).toHaveBeenCalledWith( + 'kb-1', + 'doc-1', + 'req-1', + ACCESS + ) expect(mockRecordAudit).toHaveBeenCalledWith( expect.objectContaining({ actorId: 'user-1', resourceId: 'doc-1' }) ) @@ -422,17 +429,34 @@ describe('performDeleteKnowledgeDocument', () => { }) it('emits no telemetry when the delete fails', async () => { - mockDeleteDocument.mockRejectedValue(new Error('deadlock detected')) + mockDeleteKnowledgeDocumentInKnowledgeBase.mockRejectedValue(new Error('deadlock detected')) const outcome = await performDeleteKnowledgeDocument({ ...ACTOR, knowledgeBase: KB, document: { id: 'doc-1', filename: 'report.pdf' }, + access: ACCESS, }) expect(outcome).toMatchObject({ success: false, errorCode: 'internal' }) expect(mockCaptureServerEvent).not.toHaveBeenCalled() }) + + it('reports a document the caller may no longer read as missing, and audits nothing', async () => { + mockDeleteKnowledgeDocumentInKnowledgeBase.mockRejectedValue( + new OrchestrationError('not_found', 'Document not found') + ) + + const outcome = await performDeleteKnowledgeDocument({ + ...ACTOR, + knowledgeBase: KB, + document: { id: 'doc-1', filename: 'report.pdf' }, + access: ACCESS, + }) + + expect(outcome).toMatchObject({ success: false, errorCode: 'not_found' }) + expect(mockRecordAudit).not.toHaveBeenCalled() + }) }) describe('document processing state changes', () => { diff --git a/apps/sim/lib/knowledge/orchestration/documents.ts b/apps/sim/lib/knowledge/orchestration/documents.ts index 66d1d101c5c..47a67500425 100644 --- a/apps/sim/lib/knowledge/orchestration/documents.ts +++ b/apps/sim/lib/knowledge/orchestration/documents.ts @@ -5,12 +5,13 @@ import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attr import { OrchestrationError } from '@/lib/core/orchestration/types' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' +import type { KnowledgeAccessScope } from '@/lib/knowledge/access/types' import { dispatchDocumentProcessing } from '@/lib/knowledge/documents/processing-dispatch' import { createDocumentRecords, createSingleDocument, type DocumentData, - deleteDocument, + deleteKnowledgeDocumentInKnowledgeBase, getDocumentByUploadId, markDocumentAsFailedTimeout, type ProcessingOptions, @@ -440,6 +441,13 @@ export async function performUpdateKnowledgeDocument( export interface PerformDeleteKnowledgeDocumentParams extends KnowledgeOperationContext { knowledgeBase: KnowledgeBaseTarget document: { id: string; filename: string; fileSize?: number; mimeType?: string } + /** + * The caller's read access, re-applied at the delete itself. Required rather + * than optional: the lookup that found the document is a separate statement, + * and access can be withdrawn between the two, so a delete that trusted only + * the lookup would still remove a document the caller may no longer read. + */ + access: KnowledgeAccessScope } export type PerformDeleteKnowledgeDocumentResult = KnowledgeOrchestrationResult @@ -448,11 +456,11 @@ export type PerformDeleteKnowledgeDocumentResult = KnowledgeOrchestrationResult export async function performDeleteKnowledgeDocument( params: PerformDeleteKnowledgeDocumentParams ): Promise { - const { knowledgeBase, document, request, source } = params + const { knowledgeBase, document, request, source, access } = params const requestId = params.requestId ?? generateRequestId() try { - await deleteDocument(document.id, requestId) + await deleteKnowledgeDocumentInKnowledgeBase(knowledgeBase.id, document.id, requestId, access) } catch (error) { return classifyKnowledgeFailure(error, requestId, `Delete document ${document.id}`) }