Skip to content

Commit cde2256

Browse files
committed
fix(knowledge): count documents through the reader's access and only where counts are shown
1 parent a0c93d6 commit cde2256

25 files changed

Lines changed: 339 additions & 177 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ export const GET = defineInternalJsonRoute({
3333
parseOptions: {
3434
validationErrorResponse: (error) => validationErrorResponse(error, 'Invalid query parameters'),
3535
},
36-
mapInput: ({ query }) => ({ workspaceId: query.workspaceId, scope: query.scope }),
36+
mapInput: ({ query }) => ({
37+
workspaceId: query.workspaceId,
38+
scope: query.scope,
39+
includeCounts: query.includeCounts,
40+
}),
3741
useCase: listInternalKnowledgeBases,
3842
present: internalKnowledgePresenters.list,
3943
})

‎apps/sim/app/api/v1/knowledge/[id]/route.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ import {
1414
performDeleteKnowledgeBase,
1515
performUpdateKnowledgeBase,
1616
} from '@/lib/knowledge/orchestration'
17+
import { attachKnowledgeBaseConnectors } from '@/lib/knowledge/service'
1718
import {
1819
formatKnowledgeBase,
1920
handleError,
2021
resolveKnowledgeBase,
22+
resolveV1KnowledgeReadAccess,
2123
} from '@/app/api/v1/knowledge/utils'
2224
import { authenticateRequest, v1ValidationErrorResponse } from '@/app/api/v1/middleware'
2325

@@ -41,19 +43,18 @@ export const GET = withRouteHandler(async (request: NextRequest, context: Knowle
4143
if (!parsed.success) return parsed.response
4244

4345
const { id } = parsed.data.params
44-
const result = await resolveKnowledgeBase(
45-
id,
46-
parsed.data.query.workspaceId,
47-
userId,
48-
rateLimit,
49-
'knowledge.use'
50-
)
46+
const { workspaceId } = parsed.data.query
47+
const result = await resolveKnowledgeBase(id, workspaceId, userId, rateLimit, 'knowledge.use')
5148
if (result instanceof NextResponse) return result
5249

50+
const knowledgeBase = await attachKnowledgeBaseConnectors(
51+
result.kb,
52+
await resolveV1KnowledgeReadAccess(userId, rateLimit, workspaceId)
53+
)
5354
return NextResponse.json({
5455
success: true,
5556
data: {
56-
knowledgeBase: formatKnowledgeBase(result.kb),
57+
knowledgeBase: formatKnowledgeBase(knowledgeBase),
5758
},
5859
})
5960
} catch (error) {
@@ -102,10 +103,14 @@ export const PUT = withRouteHandler(async (request: NextRequest, context: Knowle
102103
)
103104
}
104105

106+
const knowledgeBase = await attachKnowledgeBaseConnectors(
107+
outcome.knowledgeBase,
108+
await resolveV1KnowledgeReadAccess(userId, rateLimit, workspaceId)
109+
)
105110
return NextResponse.json({
106111
success: true,
107112
data: {
108-
knowledgeBase: formatKnowledgeBase(outcome.knowledgeBase),
113+
knowledgeBase: formatKnowledgeBase(knowledgeBase),
109114
message: 'Knowledge base updated successfully',
110115
},
111116
})
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* Knowledge-base document totals against real PostgreSQL: the public v1 list and detail count
3+
* only the documents their caller can read, and the internal list reads no document at all
4+
* unless the caller asks for totals.
5+
*/
6+
import type { Principal } from '@sim/auth/principal'
7+
import { db } from '@sim/db'
8+
import { document, organization, user, workspace } from '@sim/db/schema'
9+
import { createMockRequest } from '@sim/testing'
10+
import { generateId } from '@sim/utils/id'
11+
import { eq, inArray } from 'drizzle-orm'
12+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
13+
14+
const caller = vi.hoisted(() => ({ userId: '' }))
15+
16+
vi.mock('@/app/api/v1/middleware', async (importOriginal) => ({
17+
...(await importOriginal<typeof import('@/app/api/v1/middleware')>()),
18+
authenticateRequest: async () => ({
19+
requestId: 'fixture-request',
20+
userId: caller.userId,
21+
rateLimit: {
22+
allowed: true,
23+
remaining: 1,
24+
limit: 1,
25+
resetAt: new Date(),
26+
userId: caller.userId,
27+
keyType: 'personal',
28+
},
29+
}),
30+
}))
31+
32+
import {
33+
createKnowledgeAclFixtureIds,
34+
seedKnowledgeAclFixture,
35+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
36+
import { listInternalKnowledgeBases } from '@/lib/knowledge/application/knowledge-bases'
37+
import { GET as getKnowledgeBase } from '@/app/api/v1/knowledge/[id]/route'
38+
import { GET as listKnowledgeBases } from '@/app/api/v1/knowledge/route'
39+
40+
const ids = createKnowledgeAclFixtureIds()
41+
const reader: Principal = { kind: 'session', userId: ids.bobId, sessionId: 'fixture-reader' }
42+
43+
describe('knowledge-base document totals in PostgreSQL', () => {
44+
beforeAll(async () => {
45+
vi.stubGlobal('fetch', async () => {
46+
throw new Error('Unexpected provider request in knowledge-base count tests')
47+
})
48+
caller.userId = ids.bobId
49+
await seedKnowledgeAclFixture(ids, { connectorType: 'google_drive' })
50+
await db.insert(document).values([
51+
{
52+
id: generateId(),
53+
knowledgeBaseId: ids.knowledgeBaseId,
54+
filename: 'Uploaded handbook',
55+
fileUrl: 'https://fixture.test/uploaded',
56+
fileSize: 10,
57+
mimeType: 'text/plain',
58+
tokenCount: 10,
59+
processingStatus: 'completed',
60+
},
61+
{
62+
id: generateId(),
63+
knowledgeBaseId: ids.knowledgeBaseId,
64+
filename: 'Private source document',
65+
fileUrl: 'https://fixture.test/private',
66+
fileSize: 20,
67+
mimeType: 'text/plain',
68+
tokenCount: 20,
69+
processingStatus: 'completed',
70+
connectorId: ids.connectorId,
71+
externalId: 'private-fixture',
72+
contentHash: 'fixture',
73+
acl: [`u:${ids.aliceId}@fixture.test`],
74+
aclVerifiedAt: new Date(),
75+
},
76+
])
77+
})
78+
79+
afterAll(async () => {
80+
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
81+
await db.delete(organization).where(eq(organization.id, ids.organizationId))
82+
await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
83+
vi.unstubAllGlobals()
84+
})
85+
86+
it('lists only the documents a v1 caller can read', async () => {
87+
const response = await listKnowledgeBases(
88+
createMockRequest(
89+
'GET',
90+
undefined,
91+
{},
92+
`http://localhost/api/v1/knowledge?workspaceId=${ids.workspaceId}`
93+
),
94+
{ params: Promise.resolve({}) }
95+
)
96+
expect(response.status).toBe(200)
97+
const { data } = await response.json()
98+
expect(data.knowledgeBases).toEqual([
99+
expect.objectContaining({ id: ids.knowledgeBaseId, docCount: 1, tokenCount: 10 }),
100+
])
101+
})
102+
103+
it('details only the documents a v1 caller can read', async () => {
104+
const response = await getKnowledgeBase(
105+
createMockRequest(
106+
'GET',
107+
undefined,
108+
{},
109+
`http://localhost/api/v1/knowledge/${ids.knowledgeBaseId}?workspaceId=${ids.workspaceId}`
110+
),
111+
{ params: Promise.resolve({ id: ids.knowledgeBaseId }) }
112+
)
113+
expect(response.status).toBe(200)
114+
const { data } = await response.json()
115+
expect(data.knowledgeBase).toMatchObject({
116+
id: ids.knowledgeBaseId,
117+
docCount: 1,
118+
tokenCount: 10,
119+
})
120+
})
121+
122+
it('omits totals from the internal list unless the caller asks for them', async () => {
123+
const input = { workspaceId: ids.workspaceId, scope: 'active' } as const
124+
const [plain] = (await listInternalKnowledgeBases.execute({ principal: reader, input }))
125+
.knowledgeBases
126+
expect(plain).not.toHaveProperty('docCount')
127+
expect(plain).not.toHaveProperty('tokenCount')
128+
129+
const [counted] = (
130+
await listInternalKnowledgeBases.execute({
131+
principal: reader,
132+
input: { ...input, includeCounts: true },
133+
})
134+
).knowledgeBases
135+
expect(counted).toMatchObject({ docCount: 1, tokenCount: 10 })
136+
})
137+
})

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import {
1111
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1212
import { performCreateKnowledgeBase } from '@/lib/knowledge/orchestration'
1313
import { getWorkspaceKnowledgeBases } from '@/lib/knowledge/service'
14-
import { formatKnowledgeBase, handleError } from '@/app/api/v1/knowledge/utils'
14+
import {
15+
formatKnowledgeBase,
16+
handleError,
17+
resolveV1KnowledgeReadAccess,
18+
} from '@/app/api/v1/knowledge/utils'
1519
import {
1620
authenticateRequest,
1721
v1ValidationErrorResponse,
@@ -48,9 +52,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4852
)
4953
if (accessError) return accessError
5054

51-
/** Read only after `validateWorkspaceAccess` authorized this caller; same list the
52-
* internal surface serves, from the same place. */
53-
const { data: knowledgeBases } = await getWorkspaceKnowledgeBases(workspaceId)
55+
/** Read only after `validateWorkspaceAccess` authorized this caller, and totalled as the
56+
* caller reads, exactly as the v1 document routes list. */
57+
const { data: knowledgeBases } = await getWorkspaceKnowledgeBases(workspaceId, 'active', {
58+
countsFor: await resolveV1KnowledgeReadAccess(userId, rateLimit, workspaceId),
59+
})
5460

5561
return NextResponse.json({
5662
success: true,

‎apps/sim/app/api/v1/knowledge/utils.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import {
66
WORKSPACE_ACCESS_SCOPE,
77
} from '@/lib/knowledge/access/scope'
88
import type { KnowledgeAccessProvider, KnowledgeAccessScope } from '@/lib/knowledge/access/types'
9-
import { getKnowledgeBaseById } from '@/lib/knowledge/service'
9+
import type { ActiveKnowledgeBaseReference } from '@/lib/knowledge/knowledge-base-reference'
10+
import { getActiveKnowledgeBaseReference } from '@/lib/knowledge/service'
1011
import type { KnowledgeBaseWithCounts } from '@/lib/knowledge/types'
1112
import {
1213
type RateLimitResult,
@@ -32,7 +33,7 @@ export async function resolveKnowledgeBase(
3233
rateLimit: RateLimitResult,
3334
capability: V1RouteCapability,
3435
level: 'read' | 'write' = 'read'
35-
): Promise<{ kb: KnowledgeBaseWithCounts } | NextResponse> {
36+
): Promise<{ kb: ActiveKnowledgeBaseReference } | NextResponse> {
3637
const accessError = await validateWorkspaceAccess(
3738
rateLimit,
3839
userId,
@@ -42,7 +43,7 @@ export async function resolveKnowledgeBase(
4243
)
4344
if (accessError) return accessError
4445

45-
const kb = await getKnowledgeBaseById(id)
46+
const kb = await getActiveKnowledgeBaseReference(id)
4647
if (!kb) {
4748
return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 })
4849
}

‎apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,11 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) {
120120
})
121121
}
122122

123-
/** Reconciles both caches an upload moves: the base's documents and the list's `docCount`. */
123+
/** Reconciles both caches an upload moves: the base's documents and the counted `docCount`. */
124124
const invalidateKnowledgeCaches = async (knowledgeBaseId: string) => {
125125
await Promise.all([
126126
queryClient.invalidateQueries({ queryKey: knowledgeKeys.detail(knowledgeBaseId) }),
127-
queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }),
127+
queryClient.invalidateQueries({ queryKey: knowledgeKeys.countedLists() }),
128128
])
129129
}
130130

‎apps/sim/app/workspace/[workspaceId]/knowledge/knowledge.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ function KnowledgeContent() {
217217
}
218218
}, [permissionConfig.hideKnowledgeBaseTab, router, workspaceId])
219219

220-
const { knowledgeBases, isLoading, isPlaceholderData, error } = useKnowledgeBasesList(workspaceId)
220+
const { knowledgeBases, isLoading, isPlaceholderData, error } = useKnowledgeBasesList(
221+
workspaceId,
222+
{ includeCounts: true }
223+
)
221224
const { data: members } = useWorkspaceMembersQuery(workspaceId)
222225
/**
223226
* Indexed once: `ownerCell` resolves a member per row, so passing the raw array makes the

‎apps/sim/app/workspace/[workspaceId]/knowledge/prefetch.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import { prefetchResourceListChrome } from '@/app/workspace/[workspaceId]/lib/pr
1212
import { KNOWLEDGE_BASE_LIST_STALE_TIME, knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
1313

1414
/**
15-
* Prefetches the workspace's knowledge-bases list AND its knowledge-base folder tree — plus
16-
* the pinned ids and members {@link prefetchResourceListChrome} covers — under
17-
* the same query keys the client `useKnowledgeBasesQuery` / `useFolders` hooks use (scope
18-
* `active`), so the list paints populated on first render.
15+
* Prefetches the workspace's knowledge-bases list with its document totals AND its
16+
* knowledge-base folder tree — plus the pinned ids and members {@link prefetchResourceListChrome}
17+
* covers — under the same query keys the Knowledge page's `useKnowledgeBasesQuery` (scope
18+
* `active`, counted) and `useFolders` hooks use, so the list paints populated on first render.
1919
*
2020
* Both are needed: a base row is only placed correctly relative to the folder rows it sits
2121
* beside, so prefetching one without the other still flashes an ungrouped list — and a
@@ -45,12 +45,12 @@ export async function prefetchKnowledgeBases(
4545

4646
await Promise.all([
4747
queryClient.prefetchQuery({
48-
queryKey: knowledgeKeys.list(workspaceId, 'active'),
48+
queryKey: knowledgeKeys.countedList(workspaceId, 'active'),
4949
queryFn: async () => {
5050
const principal = await internalSessionAuth.authenticate()
5151
const result = await listInternalKnowledgeBases.execute({
5252
principal,
53-
input: { workspaceId, scope: 'active' },
53+
input: { workspaceId, scope: 'active', includeCounts: true },
5454
})
5555
return listKnowledgeBasesContract.response.schema.parse(
5656
internalKnowledgePresenters.list(result)

‎apps/sim/hooks/kb/use-knowledge.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ export function useKnowledgeBaseDocuments(
178178
* Hook to fetch and manage knowledge bases list
179179
* Uses React Query as single source of truth
180180
*/
181-
export function useKnowledgeBasesList(workspaceId?: string) {
182-
const query = useKnowledgeBasesQuery(workspaceId)
181+
export function useKnowledgeBasesList(workspaceId?: string, options?: { includeCounts?: boolean }) {
182+
const query = useKnowledgeBasesQuery(workspaceId, options)
183183

184184
return {
185185
knowledgeBases: query.data ?? [],

‎apps/sim/hooks/queries/kb/connectors.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ export function useUpdateConnector() {
376376
queryKey: knowledgeKeys.detail(knowledgeBaseId),
377377
exact: true,
378378
})
379-
queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() })
379+
queryClient.invalidateQueries({ queryKey: knowledgeKeys.countedLists() })
380380
queryClient.invalidateQueries({ queryKey: knowledgeKeys.searches() })
381381
}
382382
},

0 commit comments

Comments
 (0)