fix(knowledge): stop listing workspace knowledge bases on stale creator identity (#6454)

GET /api/knowledge without a workspaceId ORed on knowledge_base.user_id with no
permission check, so a user removed from a workspace kept seeing metadata for every
KB they created there. Scope the creator fallback to legacy KBs with no workspaceId,
matching the workspace-filtered branch and the detail path.

Author: waleedlatif1

apps/sim/lib/knowledge/service.test.ts

@@ -1,7 +1,15 @@
 /**
  * @vitest-environment node
  */
-import { dbChainMockFns, permissionsMock, permissionsMockFns, resetDbChainMock } from '@sim/testing'
+import {
+  dbChainMockFns,
+  flattenMockConditions,
+  hasMockCondition,
+  permissionsMock,
+  permissionsMockFns,
+  resetDbChainMock,
+  schemaMock,
+} from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
@@ -31,7 +39,63 @@ vi.mock('@/lib/billing/core/usage', () => ({
   ensureUserStatsExists: mockEnsureUserStatsExists,
 }))
 
-import { KnowledgeBasePermissionError, updateKnowledgeBase } from '@/lib/knowledge/service'
+import {
+  getKnowledgeBases,
+  KnowledgeBasePermissionError,
+  updateKnowledgeBase,
+} from '@/lib/knowledge/service'
+
+/**
+ * The listing query authorizes on current workspace membership, never on stale creator
+ * identity: a user removed from a workspace must stop seeing knowledge bases they created
+ * there. The creator fallback exists only for legacy knowledge bases with no `workspaceId`.
+ */
+describe('getKnowledgeBases — creator fallback is scoped to legacy non-workspace KBs', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+  })
+
+  /** Every disjunct that grants on `knowledgeBase.userId`, from the last select chain's WHERE. */
+  const capturedCreatorBranches = (): unknown[] => {
+    const [condition] = dbChainMockFns.where.mock.calls.at(-1) ?? []
+    const orNode = flattenMockConditions(condition).find((node) => node.type === 'or')
+    expect(orNode, 'WHERE clause has no or(...) branch').toBeDefined()
+    return (orNode?.conditions as unknown[]).filter((disjunct) =>
+      hasMockCondition(
+        disjunct,
+        (node) =>
+          node.type === 'eq' &&
+          node.left === schemaMock.knowledgeBase.userId &&
+          node.right === 'user-a'
+      )
+    )
+  }
+
+  /** The creator fallback must be the sole grant for legacy KBs and never reach workspace KBs. */
+  const expectCreatorBranchIsLegacyOnly = () => {
+    const branches = capturedCreatorBranches()
+    expect(branches).toHaveLength(1)
+    expect(
+      hasMockCondition(
+        branches[0],
+        (node) => node.type === 'isNull' && node.column === schemaMock.knowledgeBase.workspaceId
+      )
+    ).toBe(true)
+  }
+
+  it('requires workspaceId IS NULL on the creator branch when no workspace filter is given', async () => {
+    await getKnowledgeBases('user-a', undefined, 'all')
+
+    expectCreatorBranchIsLegacyOnly()
+  })
+
+  it('keeps the same guard on the workspace-filtered branch', async () => {
+    await getKnowledgeBases('user-a', 'ws-1', 'active')
+
+    expectCreatorBranchIsLegacyOnly()
+  })
+})
 
 /**
  * These tests guard the workspace mass-assignment fix:
@@ -82,7 +146,7 @@ describe('updateKnowledgeBase — workspace transfer authorization', () => {
 
     await expect(
       updateKnowledgeBase('kb-1', { workspaceId: null }, 'req-1', { actorUserId: 'owner' })
-    ).rejects.not.toBeInstanceOf(KnowledgeBasePermissionError)
+    ).resolves.toBeDefined()
     expect(permissionsMockFns.mockGetUserEntityPermissions).not.toHaveBeenCalled()
   })
 

apps/sim/lib/knowledge/service.ts

@@ -104,6 +104,21 @@ export async function getKnowledgeBases(
         ? sql`${knowledgeBase.deletedAt} IS NOT NULL`
         : isNull(knowledgeBase.deletedAt)
 
+  /**
+   * Legacy knowledge bases predate workspaces and have no `workspaceId`, so the creator is
+   * their only possible authority. Anything with a `workspaceId` must clear
+   * `currentWorkspaceMembership` instead — creator identity goes stale the moment a member
+   * is removed from the workspace.
+   */
+  const legacyOwnedKnowledgeBase = and(
+    eq(knowledgeBase.userId, userId),
+    isNull(knowledgeBase.workspaceId)
+  )
+  const currentWorkspaceMembership = and(
+    isNotNull(permissions.userId),
+    isNull(workspace.archivedAt)
+  )
+
   const knowledgeBasesWithCounts = await db
     .select({
       id: knowledgeBase.id,
@@ -143,25 +158,13 @@ export async function getKnowledgeBases(
     .where(
       and(
         scopeCondition,
-        workspaceId
-          ? // When filtering by workspace
-            or(
-              // Knowledge bases belonging to the specified workspace (user must have workspace permissions)
-              and(
-                eq(knowledgeBase.workspaceId, workspaceId),
-                isNotNull(permissions.userId),
-                isNull(workspace.archivedAt)
-              ),
-              // Fallback: User-owned knowledge bases without workspace (legacy)
-              and(eq(knowledgeBase.userId, userId), isNull(knowledgeBase.workspaceId))
-            )
-          : // When not filtering by workspace, use original logic
-            or(
-              // User owns the knowledge base directly
-              eq(knowledgeBase.userId, userId),
-              // User has permissions on the knowledge base's workspace
-              and(isNotNull(permissions.userId), isNull(workspace.archivedAt))
-            )
+        or(
+          and(
+            workspaceId ? eq(knowledgeBase.workspaceId, workspaceId) : undefined,
+            currentWorkspaceMembership
+          ),
+          legacyOwnedKnowledgeBase
+        )
       )
     )
     .groupBy(knowledgeBase.id)

packages/testing/src/mocks/database.mock.ts

@@ -29,6 +29,8 @@ export function createMockSql() {
       toSQL: () => ({ sql: strings.join('?'), params: values }),
       /** Mirrors drizzle's `sql``…`.as(alias)` for aliased select expressions. */
       as: (alias: string) => ({ ...fragment, alias }),
+      /** Mirrors drizzle's `sql``…`.mapWith(decoder)` for typed select expressions. */
+      mapWith: (decoder: unknown) => ({ ...fragment, decoder }),
     }
     return fragment
   }