Skip to content

Commit 7bac82a

Browse files
committed
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
1 parent 9573ecc commit 7bac82a

10 files changed

Lines changed: 29104 additions & 48 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)])
@@ -637,6 +648,44 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
637648
'admin-current'
638649
)
639650
).toBe(false)
651+
/**
652+
* A plan confined to one kind of source admits that kind alone, on the document and on the
653+
* row, and a plan confined to uploads admits only documents without a source.
654+
*/
655+
const drive = restrictSearchAccessPlan(plan, 'google_drive')
656+
const driveOnRow = new PgDialect().sqlToQuery(
657+
projectionCandidateAccessCondition(schema.embeddingSearch, scope, drive)
658+
)
659+
const driveOnRowAdmits = async (id: string) => {
660+
const rows = await connection.unsafe(
661+
`SELECT 1 FROM embedding_search WHERE ${driveOnRow.sql} AND document_id = $${driveOnRow.params.length + 1}`,
662+
[...(driveOnRow.params as string[]), id]
663+
)
664+
return rows.length > 0
665+
}
666+
const drivePerQuery = knowledgeCandidateAccessConditionForConnectors(scope, drive)
667+
for (const [id, expected] of [
668+
['admin-current', true],
669+
['workspace-doc', false],
670+
['members-current', false],
671+
['upload-doc', false],
672+
] as const) {
673+
expect([id, await admits(drivePerQuery, id)]).toEqual([id, expected])
674+
expect([id, await driveOnRowAdmits(id)]).toEqual([id, expected])
675+
}
676+
/** Uploads carry the workspace ACL, so a caller with that token reads them and nothing sourced. */
677+
await connection.unsafe("INSERT INTO document(id, acl) VALUES ('upload-mine', ARRAY['ws'])")
678+
const wsScope = { ...scope, tokens: [...scope.tokens, 'ws'] }
679+
const uploadsPerQuery = knowledgeCandidateAccessConditionForConnectors(
680+
wsScope,
681+
restrictSearchAccessPlan(plan, 'upload')
682+
)
683+
expect(await admits(knowledgeMetadataCandidateAccessCondition(wsScope), 'upload-mine')).toBe(
684+
true
685+
)
686+
expect(await admits(uploadsPerQuery, 'upload-mine')).toBe(true)
687+
expect(await admits(uploadsPerQuery, 'workspace-doc')).toBe(false)
688+
expect(await admits(uploadsPerQuery, 'admin-current')).toBe(false)
640689
/** A connector left out of the resolution is refused, however current its documents are. */
641690
expect(
642691
await admits(

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

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,14 @@ vi.unmock('@sim/db/schema')
1717
process.env.DATABASE_URL ??= 'postgresql://user:pass@localhost:5432/test'
1818

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

2330
function render(condition: ReturnType<typeof knowledgeAccessCondition>) {
@@ -85,3 +92,72 @@ describe('knowledgeAccessCondition', () => {
8592
expect(sql).not.toContain('"document"."acl"')
8693
})
8794
})
95+
96+
describe('restrictSearchAccessPlan', () => {
97+
const plan = {
98+
connectors: {
99+
workspace: ['slack-ws'],
100+
admin: ['drive-admin', 'confluence-admin'],
101+
members: ['slack-members'],
102+
liveProofRequired: ['confluence-admin'],
103+
},
104+
observers: {
105+
confirmed: [{ id: 'm-1', connectorId: 'slack-members' }],
106+
observed: [{ id: 'm-2', connectorId: 'drive-admin' }],
107+
},
108+
memberSources: ['slack-members'],
109+
connectorTypes: new Map([
110+
['slack-ws', 'slack'],
111+
['slack-members', 'slack'],
112+
['drive-admin', 'google_drive'],
113+
['confluence-admin', 'confluence'],
114+
]),
115+
uploads: true,
116+
}
117+
const reader = { kind: 'user' as const, userId: 'u', tokens: ['u:reader@example.com'] }
118+
119+
it('keeps only the connectors of that kind, in every list, and leaves uploads out', () => {
120+
const slack = restrictSearchAccessPlan(plan, 'slack')
121+
expect(slack.connectors).toEqual({
122+
workspace: ['slack-ws'],
123+
admin: [],
124+
members: ['slack-members'],
125+
liveProofRequired: [],
126+
})
127+
expect(slack.observers.confirmed).toEqual([{ id: 'm-1', connectorId: 'slack-members' }])
128+
expect(slack.observers.observed).toEqual([])
129+
expect(slack.memberSources).toEqual(['slack-members'])
130+
expect(slack.uploads).toBe(false)
131+
})
132+
133+
it('keeps no connector for uploads, which have none', () => {
134+
const uploads = restrictSearchAccessPlan(plan, 'upload')
135+
expect(uploads.connectors).toEqual({
136+
workspace: [],
137+
admin: [],
138+
members: [],
139+
liveProofRequired: [],
140+
})
141+
expect(uploads.memberSources).toEqual([])
142+
expect(uploads.uploads).toBe(true)
143+
})
144+
145+
it('drops source-less rows from both predicates once uploads are out of scope', () => {
146+
const rowSql = (restricted: typeof plan) =>
147+
render(
148+
projectionCandidateAccessCondition(
149+
{ connectorId: sqlColumn('connector_id'), acl: sqlColumn('acl') },
150+
reader,
151+
restricted
152+
)
153+
).sql
154+
expect(rowSql(plan)).toContain('IS NULL OR')
155+
expect(rowSql(restrictSearchAccessPlan(plan, 'slack'))).not.toContain('IS NULL')
156+
const documentSql = (restricted: typeof plan) =>
157+
render(knowledgeCandidateAccessConditionForConnectors(reader, restricted)).sql
158+
expect(documentSql(plan)).toContain('"document"."connector_id" IS NULL OR')
159+
expect(documentSql(restrictSearchAccessPlan(plan, 'slack'))).not.toContain(
160+
'"document"."connector_id" IS NULL'
161+
)
162+
})
163+
})

‎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
)
@@ -330,8 +361,10 @@ export function projectionCandidateAccessCondition(
330361
...plan.connectors.admin,
331362
...plan.connectors.members,
332363
]
333-
return sql`(${projection.acl} && ${tokens}
334-
AND (${projection.connectorId} IS NULL OR ${inSources(mirrored)}))`
364+
const owned = plan.uploads
365+
? sql`(${projection.connectorId} IS NULL OR ${inSources(mirrored)})`
366+
: inSources(mirrored)
367+
return sql`(${projection.acl} && ${tokens} AND ${owned})`
335368
}
336369

337370
/**

0 commit comments

Comments
 (0)