fix(knowledge): require workspace or organization ownership (#7635)

* fix(knowledge): require workspace or organization ownership

* fix(knowledge): update processing tests for scoped billing

Author: icecrasher321

apps/sim/app/api/v1/capability-gate.test.ts

@@ -78,7 +78,7 @@ vi.mock('@/lib/table', () => ({
 }))
 vi.mock('@/lib/table/wire', () => ({ normalizeColumn: (column: unknown) => column }))
 vi.mock('@/lib/knowledge/service', () => ({
-  listWorkspaceAndLegacyKnowledgeBases: mockListKnowledgeBases,
+  getWorkspaceKnowledgeBases: mockListKnowledgeBases,
   getKnowledgeBaseById: vi.fn(),
 }))
 vi.mock('@/lib/knowledge/orchestration', () => ({ performCreateKnowledgeBase: vi.fn() }))
@@ -158,7 +158,7 @@ beforeEach(() => {
   mockGetUserEntityPermissions.mockResolvedValue('admin')
   mockGetWorkspaceBillingSettings.mockResolvedValue({ allowPersonalApiKeys: true })
   mockListTables.mockResolvedValue([])
-  mockListKnowledgeBases.mockResolvedValue([])
+  mockListKnowledgeBases.mockResolvedValue({ data: [], nextCursorKeys: null })
   mockListWorkspaceFiles.mockResolvedValue([])
   mockListPublicWorkflowLogs.mockResolvedValue({ data: [], nextCursor: null })
   mockGetDeploymentWorkflowTarget.mockResolvedValue({

apps/sim/app/api/v1/knowledge/route.ts

@@ -10,7 +10,7 @@ import {
 } from '@/lib/core/orchestration/types'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { performCreateKnowledgeBase } from '@/lib/knowledge/orchestration'
-import { listWorkspaceAndLegacyKnowledgeBases } from '@/lib/knowledge/service'
+import { getWorkspaceKnowledgeBases } from '@/lib/knowledge/service'
 import { formatKnowledgeBase, handleError } from '@/app/api/v1/knowledge/utils'
 import {
   authenticateRequest,
@@ -50,7 +50,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
     /** Read only after `validateWorkspaceAccess` authorized this caller; same list the
      *  internal surface serves, from the same place. */
-    const knowledgeBases = await listWorkspaceAndLegacyKnowledgeBases(userId, workspaceId)
+    const { data: knowledgeBases } = await getWorkspaceKnowledgeBases(workspaceId)
 
     return NextResponse.json({
       success: true,

apps/sim/lib/knowledge/__integration__/organization-mcp-search.integration.ts

@@ -64,6 +64,8 @@ import {
 import { confluencePageAcl } from '@/lib/knowledge/access/confluence-permissions'
 import { listKnowledgeChunks } from '@/lib/knowledge/application/chunks'
 import { readKnowledgeDocument } from '@/lib/knowledge/application/documents'
+import { listKnowledgeBaseCatalog } from '@/lib/knowledge/application/knowledge-bases'
+import { prepareSearchSource } from '@/lib/knowledge/application/sim-search'
 import { searchScopedKnowledge } from '@/lib/knowledge/application/workspace-search'
 import { createContentSyncLease } from '@/lib/knowledge/connectors/sync-lock'
 import { addDocument, persistDocumentAcls } from '@/lib/knowledge/connectors/sync-persistence'
@@ -84,8 +86,9 @@ describe('organization Search MCP with real ingestion and current access', () =>
     groupIds,
   } = ids
   const otherOrganizationId = generateId()
-  const otherKnowledgeBaseId = generateId()
+  const workspaceKnowledgeBaseId = generateId()
   const outsiderId = generateId()
+  const otherAdminId = generateId()
   const bobMembershipId = generateId()
   const tokens = {
     alice: generateId(),
@@ -98,10 +101,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
   const clients: Client[] = []
   const alicePrincipal: Principal = { kind: 'session', userId: aliceId, sessionId: generateId() }
   const bobPrincipal: Principal = { kind: 'session', userId: bobId, sessionId: generateId() }
+  const otherAdminPrincipal: Principal = {
+    kind: 'session',
+    userId: otherAdminId,
+    sessionId: generateId(),
+  }
   const bobSourceMembership = {
     groupId: groupIds[2],
     subjectToken: `u:${bobId}@fixture.test`,
   }
+  let otherKnowledgeBaseId: string
   let documentId: string
   let alice: Client
   let bob: Client
@@ -210,14 +219,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
     })
     fixtures.storageRoot = mkdtempSync(path.join(tmpdir(), 'sim-organization-mcp-integration-'))
     await seedKnowledgeAclFixture(ids)
-    await db.insert(user).values({
-      id: outsiderId,
-      name: 'Other organization fixture',
-      email: `${outsiderId}@fixture.test`,
-      emailVerified: true,
-      createdAt: new Date(),
-      updatedAt: new Date(),
-    })
+    await db.insert(user).values(
+      [outsiderId, otherAdminId].map((id) => ({
+        id,
+        name: 'Other organization fixture',
+        email: `${id}@fixture.test`,
+        emailVerified: true,
+        createdAt: new Date(),
+        updatedAt: new Date(),
+      }))
+    )
     await db.insert(organization).values({
       id: otherOrganizationId,
       name: 'Other organization MCP fixture',
@@ -228,6 +239,12 @@ describe('organization Search MCP with real ingestion and current access', () =>
       { id: generateId(), userId: aliceId, organizationId, role: 'owner' },
       { id: bobMembershipId, userId: bobId, organizationId, role: 'member' },
       { id: generateId(), userId: outsiderId, organizationId: otherOrganizationId, role: 'owner' },
+      {
+        id: generateId(),
+        userId: otherAdminId,
+        organizationId: otherOrganizationId,
+        role: 'admin',
+      },
     ])
     /** Reuse source identities, but establish exclusive organization ownership before ingestion. */
     await db
@@ -244,12 +261,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
       .update(knowledgeExternalGroup)
       .set({ workspaceId: null, organizationId })
       .where(inArray(knowledgeExternalGroup.id, groupIds))
+    const prepared = await prepareSearchSource.execute({
+      principal: otherAdminPrincipal,
+      input: { organizationId: otherOrganizationId, connectorType: 'gitlab' },
+    })
+    otherKnowledgeBaseId = prepared.knowledgeBaseId
     await db.insert(knowledgeBase).values({
-      id: otherKnowledgeBaseId,
-      userId: outsiderId,
-      organizationId: otherOrganizationId,
-      isSearchIndex: true,
-      name: 'Other org index',
+      id: workspaceKnowledgeBaseId,
+      userId: bobId,
+      workspaceId,
+      name: 'Workspace documents',
     })
     await db.insert(organizationSearchIntegration).values({
       organizationId,
@@ -363,12 +384,56 @@ describe('organization Search MCP with real ingestion and current access', () =>
       .delete(organization)
       .where(inArray(organization.id, [organizationId, otherOrganizationId]))
     await db.delete(workspace).where(eq(workspace.id, workspaceId))
-    await db.delete(user).where(inArray(user.id, [aliceId, bobId, outsiderId]))
+    await db.delete(user).where(inArray(user.id, [aliceId, bobId, outsiderId, otherAdminId]))
     if (fixtures.storageRoot) await rm(fixtures.storageRoot, { recursive: true, force: true })
     await db.$client.end()
     vi.unstubAllGlobals()
   })
 
+  it('creates an organization-only index, keeps it out of the workspace catalog, and separates actor from payer', async () => {
+    const input = { organizationId: otherOrganizationId, connectorType: 'gitlab' }
+    const results = await Promise.all([
+      prepareSearchSource.execute({ principal: otherAdminPrincipal, input }),
+      prepareSearchSource.execute({ principal: otherAdminPrincipal, input }),
+    ])
+    expect(results.map((result) => result.knowledgeBaseId)).toEqual([
+      otherKnowledgeBaseId,
+      otherKnowledgeBaseId,
+    ])
+    const indexes = await db
+      .select()
+      .from(knowledgeBase)
+      .where(eq(knowledgeBase.organizationId, otherOrganizationId))
+    expect(indexes).toEqual([
+      expect.objectContaining({
+        id: otherKnowledgeBaseId,
+        workspaceId: null,
+        organizationId: otherOrganizationId,
+        isSearchIndex: true,
+        userId: otherAdminId,
+      }),
+    ])
+    const catalog = await listKnowledgeBaseCatalog.execute({
+      principal: alicePrincipal,
+      input: { workspaceId },
+    })
+    expect(catalog.knowledgeBases.map(({ knowledgeBase }) => knowledgeBase.id)).toEqual([
+      workspaceKnowledgeBaseId,
+    ])
+    await expect(
+      resolveOrganizationBillingAttribution({
+        actorUserId: otherAdminId,
+        organizationId: otherOrganizationId,
+      })
+    ).resolves.toMatchObject({
+      actorUserId: otherAdminId,
+      workspaceId: null,
+      organizationId: otherOrganizationId,
+      billedAccountUserId: outsiderId,
+      billingEntity: { type: 'organization', id: otherOrganizationId },
+    })
+  })
+
   it('finds the canonical org-owned index and applies each current member’s source ACL to all tools', async () => {
     const [owner] = await db
       .select({

apps/sim/lib/knowledge/__integration__/storage-cleanup.integration.ts

@@ -30,7 +30,7 @@ import {
   createKnowledgeAclFixtureIds,
   seedKnowledgeAclFixture,
 } from '@/lib/knowledge/__integration__/seed-source-access-fixture'
-import { createSingleDocument, hardDeleteDocuments } from '@/lib/knowledge/documents/service'
+import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
 import {
   cleanupKnowledgeStorage,
   enqueueKnowledgeStorageCleanup,
@@ -245,59 +245,42 @@ describe('knowledge backing storage cleanup in PostgreSQL', () => {
     }
   })
 
-  it('cleans a legacy personal KB document using its canonical user-owned binding', async () => {
-    const fixture = await seed()
-    await db
-      .update(knowledgeBase)
-      .set({ workspaceId: null })
-      .where(eq(knowledgeBase.id, fixture.knowledgeBaseId))
-    await db
-      .update(workspaceFiles)
-      .set({ workspaceId: null })
-      .where(eq(workspaceFiles.id, fixture.binding.id))
-    expect(await hardDeleteDocuments([fixture.docId], 'personal-cleanup')).toBe(1)
-    const event = await cleanupEvent(fixture.docId)
-    expect(event.payload).toMatchObject({
-      userId: fixture.aliceId,
-      workspaceId: null,
-      organizationId: null,
-    })
-    expect(await processOutboxEventById(event.id, handlers)).toBe('completed')
-    expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toEqual([])
-    await expect(access(fixture.filePath)).rejects.toMatchObject({ code: 'ENOENT' })
-    await expect(
-      createSingleDocument(
-        {
-          filename: 'expired.txt',
-          fileUrl: fixture.fileUrl,
-          fileSize: 25,
-          mimeType: 'text/plain',
+  it.each([false, true])(
+    'handles already-queued personal cleanup after KB ownership repair (owner changed: %s)',
+    async (ownerChanged) => {
+      const fixture = await seed()
+      await db.delete(document).where(eq(document.id, fixture.docId))
+      await db
+        .update(workspaceFiles)
+        .set({ workspaceId: null, userId: ownerChanged ? fixture.bobId : fixture.aliceId })
+        .where(eq(workspaceFiles.id, fixture.binding.id))
+      const eventId = `knowledge-storage-cleanup:${generateId()}`
+      events.push(eventId)
+      await db.insert(outboxEvent).values({
+        id: eventId,
+        eventType: KNOWLEDGE_STORAGE_CLEANUP_EVENT,
+        payload: {
+          version: 1,
+          documentId: fixture.docId,
+          fileId: fixture.binding.id,
+          key: fixture.key,
+          contentUpdatedAt: fixture.binding.contentUpdatedAt.toISOString(),
+          userId: fixture.aliceId,
+          workspaceId: null,
+          organizationId: null,
         },
-        fixture.knowledgeBaseId,
-        'personal-expired-upload',
-        fixture.aliceId
-      )
-    ).rejects.toThrow('not owned')
-  })
+      })
 
-  it('rolls back personal document deletion when the file belongs to a different user', async () => {
-    const fixture = await seed()
-    await db
-      .update(knowledgeBase)
-      .set({ workspaceId: null })
-      .where(eq(knowledgeBase.id, fixture.knowledgeBaseId))
-    await db
-      .update(workspaceFiles)
-      .set({ workspaceId: null, userId: fixture.bobId })
-      .where(eq(workspaceFiles.id, fixture.binding.id))
-    await expect(hardDeleteDocuments([fixture.docId], 'personal-mismatch')).rejects.toThrow(
-      'ownership binding'
-    )
-    expect(
-      await db.select({ id: document.id }).from(document).where(eq(document.id, fixture.docId))
-    ).toHaveLength(1)
-    await expect(access(fixture.filePath)).resolves.toBeUndefined()
-  })
+      expect(await processOutboxEventById(eventId, handlers)).toBe('completed')
+      if (ownerChanged) {
+        expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toHaveLength(1)
+        await expect(access(fixture.filePath)).resolves.toBeUndefined()
+      } else {
+        expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toEqual([])
+        await expect(access(fixture.filePath)).rejects.toMatchObject({ code: 'ENOENT' })
+      }
+    }
+  )
 
   it('allows a create-only re-upload to register a new version after cleanup tombstones its old binding', async () => {
     const fixture = await seed()

apps/sim/lib/knowledge/access/scope.test.ts

@@ -143,12 +143,10 @@ describe('resolveKnowledgeAccessScope', () => {
     })
   })
 
-  it('does not query for a legacy personal knowledge base', async () => {
-    await expect(resolveKnowledgeAccessScope(SESSION, {})).resolves.toEqual({
-      kind: 'user',
-      userId: 'user-1',
-      tokens: ['pub', 'ws'],
-    })
+  it('rejects missing ownership before querying document access', async () => {
+    await expect(resolveKnowledgeAccessScope(SESSION, {})).rejects.toThrow(
+      'Resource requires exactly one workspace or organization owner'
+    )
     expect(dbChainMockFns.select).not.toHaveBeenCalled()
   })
 

apps/sim/lib/knowledge/access/scope.ts

@@ -117,7 +117,7 @@ async function loadExternalGroupTokens(
 }
 
 export interface KnowledgeAccessScopeContext {
-  /** Undefined only for a legacy personal knowledge base, which cannot own connectors. */
+  /** Exactly one workspace or organization owner is required at resolution. */
   workspaceId?: string
   organizationId?: string
 }
@@ -136,7 +136,6 @@ async function loadUserAccessTokens(
   context: KnowledgeAccessScopeContext
 ): Promise<string[]> {
   const { workspaceId, organizationId } = context
-  if (!workspaceId && !organizationId) return [...WORKSPACE_ACCESS_TOKENS]
   const scope = resourceScopeFromOwner(context)
   const baseline = organizationId ? ORGANIZATION_ACCESS_TOKENS : WORKSPACE_ACCESS_TOKENS
 
@@ -277,6 +276,7 @@ export async function resolveKnowledgeAccessScope(
       'Credential Group enrollments cannot read knowledge documents'
     )
   }
+  resourceScopeFromOwner(context)
   const subject = resolvePrincipalSubject(principal)
   if (subject?.kind !== 'sim_user') {
     if (context.organizationId)

apps/sim/lib/knowledge/application/authorization.ts

@@ -20,12 +20,6 @@ export interface KnowledgeAuthorizationContext
   organizationId?: undefined
 }
 
-export interface LegacyPersonalKnowledgeAuthorizationContext extends KnowledgeResourceIdentifiers {
-  workspaceId: undefined
-  organizationId?: undefined
-  legacyPersonalOwnerUserId: string
-}
-
 export interface KnowledgeOrganizationAuthorizationContext extends KnowledgeResourceIdentifiers {
   organizationId: string
   workspaceId: undefined
@@ -34,7 +28,6 @@ export interface KnowledgeOrganizationAuthorizationContext extends KnowledgeReso
 export type KnowledgeResourceAuthorizationContext =
   | KnowledgeAuthorizationContext
   | KnowledgeOrganizationAuthorizationContext
-  | LegacyPersonalKnowledgeAuthorizationContext
 
 export type KnowledgeAuthorizationOptions = Omit<
   WorkspaceAuthorizationOptions<KnowledgeAuthorizationContext>,