Skip to content

Commit 7093409

Browse files
authored
fix(knowledge): apply the source and date filters inside a resolved scope's ranking (#8059)
* fix(knowledge): apply the source and date filters inside a resolved scope's ranking - a source filter confines the access plan itself, so every on-row predicate, the reach, and the sources the legs walk or rank are that kind of source alone; `upload` keeps only source-less documents - a date filter enumerates the documents it admits off a new `(knowledge_base_id, source_modified_at)` index and ranks them exactly while the planner estimates the window within the probe's limit; a wider window is walked with the date tested through the document, on the row and on the ranked keyword row - a date-bounded set is ranked exactly even when a member source has its own index, since a walk cannot see the date - the reach is keyed and counted by the plan's sources - a keyword leg whose deadline passes before its ranking is resolved is short rather than failed * fix(knowledge): enumerate a small filtered source so both legs rank inside it * chore(db): mark the document date lookup index concurrent in the schema * fix(knowledge): estimate a filter's size under a short deadline of its own
1 parent a41ef97 commit 7093409

10 files changed

Lines changed: 29213 additions & 52 deletions

File tree

‎apps/sim/lib/knowledge/access/connector-eligibility.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ import { searchIntegrationAccessCondition } from '@/lib/knowledge/search/integra
2222
*/
2323
async function resolveConnectorEligibility(
2424
knowledgeBaseIds: readonly string[]
25-
): Promise<KnowledgeConnectorEligibility> {
25+
): Promise<{ eligibility: KnowledgeConnectorEligibility; types: Map<string, string> }> {
2626
const eligibility: {
2727
workspace: string[]
2828
admin: string[]
2929
members: string[]
3030
liveProofRequired: string[]
3131
} = { workspace: [], admin: [], members: [], liveProofRequired: [] }
32-
if (knowledgeBaseIds.length === 0) return eligibility
32+
const types = new Map<string, string>()
33+
if (knowledgeBaseIds.length === 0) return { eligibility, types }
3334
const rows = await db
3435
.select({
3536
id: knowledgeConnector.id,
@@ -53,12 +54,13 @@ async function resolveConnectorEligibility(
5354
else if (row.accessMode === 'admin') eligibility.admin.push(row.id)
5455
else if (row.accessMode === 'members') eligibility.members.push(row.id)
5556
else continue
57+
types.set(row.id, row.connectorType)
5658
const live =
5759
(row.connectorType === 'github' && row.githubRepository) ||
5860
(row.connectorType === 'confluence' && row.accessMode === 'admin')
5961
if (live) eligibility.liveProofRequired.push(row.id)
6062
}
61-
return eligibility
63+
return { eligibility, types }
6264
}
6365

6466
/**
@@ -116,7 +118,8 @@ export async function resolveSearchAccessPlan(
116118
knowledgeBaseIds: readonly string[],
117119
access: KnowledgeAccessScope
118120
): Promise<SearchAccessPlan> {
119-
const connectors = await resolveConnectorEligibility(knowledgeBaseIds)
121+
const { eligibility: connectors, types: connectorTypes } =
122+
await resolveConnectorEligibility(knowledgeBaseIds)
120123
const { observers, memberSources } = await resolveMemberObservers(access, connectors.members)
121-
return { connectors, observers, memberSources }
124+
return { connectors, observers, memberSources, connectorTypes, uploads: true }
122125
}

‎apps/sim/lib/knowledge/access/predicate.postgres.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const {
3030
knowledgeAccessCondition,
3131
knowledgeCandidateAccessConditionForConnectors,
3232
projectionCandidateAccessCondition,
33+
restrictSearchAccessPlan,
3334
knowledgeMetadataCandidateAccessCondition,
3435
} = await import('@/lib/knowledge/access/predicate')
3536
const { confluencePageAcl } = await import('@/lib/knowledge/access/confluence-permissions')
@@ -557,7 +558,17 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
557558
/** What `resolveSearchAccessPlan` resolves for this caller: their member identity, confirmed. */
558559
const observers = { confirmed: [{ id: 'm-alice', connectorId: 'members' }], observed: [] }
559560
const perRow = knowledgeMetadataCandidateAccessCondition(scope)
560-
const plan = { connectors: eligibility, observers, memberSources: ['members'] }
561+
const plan = {
562+
connectors: eligibility,
563+
observers,
564+
memberSources: ['members'],
565+
connectorTypes: new Map([
566+
['ws-mode', 'slack'],
567+
['admin', 'google_drive'],
568+
['members', 'slack'],
569+
]),
570+
uploads: true,
571+
}
561572
const perQuery = knowledgeCandidateAccessConditionForConnectors(scope, plan)
562573
for (const id of [...cases.map(([documentId]) => documentId), 'upload-doc']) {
563574
expect([id, await admits(perQuery, id)]).toEqual([id, await admits(perRow, id)])
@@ -655,6 +666,44 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
655666
'admin-current'
656667
)
657668
).toBe(false)
669+
/**
670+
* A plan confined to one kind of source admits that kind alone, on the document and on the
671+
* row, and a plan confined to uploads admits only documents without a source.
672+
*/
673+
const drive = restrictSearchAccessPlan(plan, 'google_drive')
674+
const driveOnRow = new PgDialect().sqlToQuery(
675+
projectionCandidateAccessCondition(schema.embeddingSearch, scope, drive)
676+
)
677+
const driveOnRowAdmits = async (id: string) => {
678+
const rows = await connection.unsafe(
679+
`SELECT 1 FROM embedding_search WHERE ${driveOnRow.sql} AND document_id = $${driveOnRow.params.length + 1}`,
680+
[...(driveOnRow.params as string[]), id]
681+
)
682+
return rows.length > 0
683+
}
684+
const drivePerQuery = knowledgeCandidateAccessConditionForConnectors(scope, drive)
685+
for (const [id, expected] of [
686+
['admin-current', true],
687+
['workspace-doc', false],
688+
['members-current', false],
689+
['upload-doc', false],
690+
] as const) {
691+
expect([id, await admits(drivePerQuery, id)]).toEqual([id, expected])
692+
expect([id, await driveOnRowAdmits(id)]).toEqual([id, expected])
693+
}
694+
/** Uploads carry the workspace ACL, so a caller with that token reads them and nothing sourced. */
695+
await connection.unsafe("INSERT INTO document(id, acl) VALUES ('upload-mine', ARRAY['ws'])")
696+
const wsScope = { ...scope, tokens: [...scope.tokens, 'ws'] }
697+
const uploadsPerQuery = knowledgeCandidateAccessConditionForConnectors(
698+
wsScope,
699+
restrictSearchAccessPlan(plan, 'upload')
700+
)
701+
expect(await admits(knowledgeMetadataCandidateAccessCondition(wsScope), 'upload-mine')).toBe(
702+
true
703+
)
704+
expect(await admits(uploadsPerQuery, 'upload-mine')).toBe(true)
705+
expect(await admits(uploadsPerQuery, 'workspace-doc')).toBe(false)
706+
expect(await admits(uploadsPerQuery, 'admin-current')).toBe(false)
658707
/** A connector left out of the resolution is refused, however current its documents are. */
659708
expect(
660709
await admits(

‎apps/sim/lib/knowledge/access/predicate.test.ts‎

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,14 @@ process.env.DATABASE_URL ??= 'postgresql://user:pass@localhost:5432/test'
1818

1919
const { PgDialect } = await import('drizzle-orm/pg-core')
2020
const { embeddingSearch } = await import('@sim/db/schema')
21-
const { knowledgeAccessCondition, projectionCandidateAccessCondition } = await import(
22-
'@/lib/knowledge/access/predicate'
23-
)
21+
const { sql: rawSql } = await import('drizzle-orm')
22+
const sqlColumn = (name: string) => rawSql.raw(`"row"."${name}"`)
23+
const {
24+
knowledgeAccessCondition,
25+
knowledgeCandidateAccessConditionForConnectors,
26+
projectionCandidateAccessCondition,
27+
restrictSearchAccessPlan,
28+
} = await import('@/lib/knowledge/access/predicate')
2429
const { SYSTEM_ACCESS_SCOPE } = await import('@/lib/knowledge/access/types')
2530

2631
function render(condition: ReturnType<typeof knowledgeAccessCondition>) {
@@ -32,6 +37,8 @@ describe('projectionCandidateAccessCondition', () => {
3237
connectors: { workspace: ['ws-src'], admin: [], members: [], liveProofRequired: [] },
3338
observers: { confirmed: [], observed: [] },
3439
memberSources: [],
40+
connectorTypes: new Map(),
41+
uploads: true,
3542
}
3643

3744
it('decides a filled row on its mirrored columns and an unfilled row on its document', () => {
@@ -46,8 +53,9 @@ describe('projectionCandidateAccessCondition', () => {
4653
'("embedding_search"."acl" IS NULL AND EXISTS (\n SELECT 1 FROM "document"\n WHERE "document"."id" = "embedding_search"."document_id"\n AND ('
4754
)
4855
expect(sql).toContain('"document"."acl" && ARRAY[$1, $2]::text[]')
49-
expect(sql).toMatch(
50-
/OR \("embedding_search"\."acl" && ARRAY\[\$\d+, \$\d+\]::text\[\]\n {4}AND \("embedding_search"\."connector_id" IS NULL OR "embedding_search"\."connector_id" = ANY\(ARRAY\[\$\d+\]::text\[\]\)\)\)\)$/
56+
expect(sql).toContain('OR ("embedding_search"."acl" && ARRAY[')
57+
expect(sql).toContain(
58+
'AND ("embedding_search"."connector_id" IS NULL OR "embedding_search"."connector_id" = ANY(ARRAY['
5159
)
5260
expect(params.slice(0, 2)).toEqual(['ws', 'u:alice'])
5361
expect(params.slice(-3)).toEqual(['ws', 'u:alice', 'ws-src'])
@@ -128,3 +136,78 @@ describe('knowledgeAccessCondition', () => {
128136
expect(sql).not.toContain('"document"."acl"')
129137
})
130138
})
139+
140+
describe('restrictSearchAccessPlan', () => {
141+
const plan = {
142+
connectors: {
143+
workspace: ['slack-ws'],
144+
admin: ['drive-admin', 'confluence-admin'],
145+
members: ['slack-members'],
146+
liveProofRequired: ['confluence-admin'],
147+
},
148+
observers: {
149+
confirmed: [{ id: 'm-1', connectorId: 'slack-members' }],
150+
observed: [{ id: 'm-2', connectorId: 'drive-admin' }],
151+
},
152+
memberSources: ['slack-members'],
153+
connectorTypes: new Map([
154+
['slack-ws', 'slack'],
155+
['slack-members', 'slack'],
156+
['drive-admin', 'google_drive'],
157+
['confluence-admin', 'confluence'],
158+
]),
159+
uploads: true,
160+
}
161+
const reader = { kind: 'user' as const, userId: 'u', tokens: ['u:reader@example.com'] }
162+
163+
it('keeps only the connectors of that kind, in every list, and leaves uploads out', () => {
164+
const slack = restrictSearchAccessPlan(plan, 'slack')
165+
expect(slack.connectors).toEqual({
166+
workspace: ['slack-ws'],
167+
admin: [],
168+
members: ['slack-members'],
169+
liveProofRequired: [],
170+
})
171+
expect(slack.observers.confirmed).toEqual([{ id: 'm-1', connectorId: 'slack-members' }])
172+
expect(slack.observers.observed).toEqual([])
173+
expect(slack.memberSources).toEqual(['slack-members'])
174+
expect(slack.uploads).toBe(false)
175+
})
176+
177+
it('keeps no connector for uploads, which have none', () => {
178+
const uploads = restrictSearchAccessPlan(plan, 'upload')
179+
expect(uploads.connectors).toEqual({
180+
workspace: [],
181+
admin: [],
182+
members: [],
183+
liveProofRequired: [],
184+
})
185+
expect(uploads.memberSources).toEqual([])
186+
expect(uploads.uploads).toBe(true)
187+
})
188+
189+
it('drops source-less rows from both predicates once uploads are out of scope', () => {
190+
const rowSql = (restricted: typeof plan) =>
191+
render(
192+
projectionCandidateAccessCondition(
193+
{
194+
connectorId: sqlColumn('connector_id'),
195+
acl: sqlColumn('acl'),
196+
documentId: sqlColumn('document_id'),
197+
},
198+
reader,
199+
restricted
200+
)
201+
).sql
202+
expect(rowSql(plan)).toContain('IS NULL OR')
203+
expect(rowSql(restrictSearchAccessPlan(plan, 'slack'))).not.toContain(
204+
'"row"."connector_id" IS NULL'
205+
)
206+
const documentSql = (restricted: typeof plan) =>
207+
render(knowledgeCandidateAccessConditionForConnectors(reader, restricted)).sql
208+
expect(documentSql(plan)).toContain('"document"."connector_id" IS NULL OR')
209+
expect(documentSql(restrictSearchAccessPlan(plan, 'slack'))).not.toContain(
210+
'"document"."connector_id" IS NULL'
211+
)
212+
})
213+
})

‎apps/sim/lib/knowledge/access/predicate.ts‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,35 @@ export interface SearchAccessPlan {
232232
observers: KnowledgeMemberObservers
233233
/** Connectors the caller is an active member of, whose documents they read broadly. */
234234
memberSources: readonly string[]
235+
/** Each eligible connector's type, so a search may be confined to one kind of source. */
236+
connectorTypes: ReadonlyMap<string, string>
237+
/** Whether documents without a source — uploads — are in scope. */
238+
uploads: boolean
239+
}
240+
241+
/**
242+
* The plan confined to one kind of source: the connectors of that type keep their eligibility and
243+
* the rest lose it, so every predicate built from the plan — on the row and on the document — and
244+
* every source the legs walk or rank are that kind alone. `upload` keeps only source-less documents.
245+
*/
246+
export function restrictSearchAccessPlan(plan: SearchAccessPlan, source: string): SearchAccessPlan {
247+
const keep = (id: string) => source !== 'upload' && plan.connectorTypes.get(id) === source
248+
const kept = (ids: readonly string[]) => ids.filter(keep)
249+
return {
250+
connectors: {
251+
workspace: kept(plan.connectors.workspace),
252+
admin: kept(plan.connectors.admin),
253+
members: kept(plan.connectors.members),
254+
liveProofRequired: kept(plan.connectors.liveProofRequired),
255+
},
256+
observers: {
257+
confirmed: plan.observers.confirmed.filter((observer) => keep(observer.connectorId)),
258+
observed: plan.observers.observed.filter((observer) => keep(observer.connectorId)),
259+
},
260+
memberSources: kept(plan.memberSources),
261+
connectorTypes: plan.connectorTypes,
262+
uploads: source === 'upload',
263+
}
235264
}
236265

237266
export interface KnowledgeConnectorEligibility {
@@ -290,12 +319,14 @@ export function knowledgeCandidateAccessConditionForConnectors(
290319
))
291320
)`
292321
}
322+
const workspaceOwned = plan.uploads
323+
? sql`(${document.connectorId} IS NULL OR ${inConnectors(eligibility.workspace)})`
324+
: inConnectors(eligibility.workspace)
293325
return sql`(
294326
${aclOverlap(tokens)}
295327
AND ${aclRequirementsSatisfied(tokens)}
296328
AND (
297-
((${document.connectorId} IS NULL OR ${inConnectors(eligibility.workspace)})
298-
AND ${document.acl} = ARRAY['ws']::text[])
329+
(${workspaceOwned} AND ${document.acl} = ARRAY['ws']::text[])
299330
OR ${mirrored(eligibility.admin, sql`${document.aclVerifiedAt} > ${cutoff}`)}
300331
OR ${mirrored(eligibility.members, resolvedObservationCondition(plan.observers, cutoff))}
301332
)
@@ -340,13 +371,15 @@ export function projectionCandidateAccessCondition(
340371
...plan.connectors.admin,
341372
...plan.connectors.members,
342373
]
374+
const owned = plan.uploads
375+
? sql`(${projection.connectorId} IS NULL OR ${inSources(mirrored)})`
376+
: inSources(mirrored)
343377
const unfilled = sql`(${projection.acl} IS NULL AND EXISTS (
344378
SELECT 1 FROM ${document}
345379
WHERE ${document.id} = ${projection.documentId}
346380
AND ${knowledgeCandidateAccessConditionForConnectors(scope, plan)}
347381
))`
348-
return sql`(${unfilled} OR (${projection.acl} && ${tokens}
349-
AND (${projection.connectorId} IS NULL OR ${inSources(mirrored)})))`
382+
return sql`(${unfilled} OR (${projection.acl} && ${tokens} AND ${owned}))`
350383
}
351384

352385
/**

0 commit comments

Comments
 (0)