Skip to content

Commit 49c06dd

Browse files
committed
fix(search): bind live IDs to their reader through the checksum, not a visible field
1 parent 293d58c commit 49c06dd

8 files changed

Lines changed: 141 additions & 117 deletions

File tree

‎apps/sim/lib/knowledge/__integration__/gitlab-federated.integration.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,10 @@ describe.skipIf(!fixtureFile)('federated self-hosted GitLab Search', () => {
135135

136136
function documentForReader(documentId: string, person: FixturePerson) {
137137
const scope = resourceScopeKey(resourceScopeFromOwner({ organizationId: ids.organizationId }))
138-
return encodeLiveReference({
139-
...decodeLiveReference(documentId),
140-
owner: liveReferenceOwner(person.simId, scope),
141-
})
138+
return encodeLiveReference(
139+
decodeLiveReference(documentId),
140+
liveReferenceOwner(person.simId, scope)
141+
)
142142
}
143143

144144
beforeAll(async () => {

‎apps/sim/lib/knowledge/mcp/server.test.ts‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ vi.mock('@/lib/knowledge/application/chat', () => ({
5555

5656
import { OrchestrationError } from '@/lib/core/orchestration/types'
5757
import { createKnowledgeMcpServer } from '@/lib/knowledge/mcp/server'
58-
import { encodeLiveReference, liveReferenceOwner } from '@/lib/sim-search/live/reference'
5958
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
6059

6160
const mocks = hoisted
@@ -302,24 +301,6 @@ describe('batched document reads', () => {
302301
expect(mocks.liveRead).toHaveBeenCalledTimes(2)
303302
})
304303

305-
it('reads a document once when the batch names it in both ID forms', async () => {
306-
create()
307-
mocks.liveRead.mockImplementation(async ({ input }: { input: { documentId: string } }) =>
308-
readResult(input.documentId)
309-
)
310-
const reference = { provider: 'google_drive', account: 'account', id: 'doc' } as const
311-
const earlier = `live:${Buffer.from(
312-
JSON.stringify({ v: 1, user: 'person-1', scope: 'organization:org-1', ...reference })
313-
).toString('base64url')}`
314-
const current = encodeLiveReference({
315-
...reference,
316-
owner: liveReferenceOwner('person-1', 'organization:org-1'),
317-
})
318-
await call('read_document', { documentIds: [earlier, current] })
319-
expect(mocks.rateLimit).toHaveBeenCalledWith(request, auth, expect.anything(), 1)
320-
expect(mocks.liveRead).toHaveBeenCalledTimes(1)
321-
})
322-
323304
it('reads at most five documents from providers at once, in request order', async () => {
324305
create()
325306
let inFlight = 0

‎apps/sim/lib/knowledge/mcp/server.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import { liveCitationId } from '@/lib/knowledge/search/citation'
3030
import { toolError } from '@/lib/mcp/tool-result'
3131
import { readLiveDocument, searchLiveKnowledge } from '@/lib/sim-search/live/application'
3232
import { LiveReadError } from '@/lib/sim-search/live/read-error'
33-
import { liveDocumentKey } from '@/lib/sim-search/live/reference'
3433
import { v2CaughtOrchestrationError } from '@/app/api/v2/lib/response'
3534
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
3635

@@ -226,9 +225,7 @@ export function createKnowledgeMcpServer(context: KnowledgeMcpContext): McpServe
226225
async (raw: unknown, extra: { signal: AbortSignal }) => {
227226
const parsed = readLiveDocumentMcpSchema.safeParse(raw)
228227
const batch =
229-
parsed.success && parsed.data.documentIds
230-
? [...new Map(parsed.data.documentIds.map((id) => [liveDocumentKey(id), id])).values()]
231-
: []
228+
parsed.success && parsed.data.documentIds ? [...new Set(parsed.data.documentIds)] : []
232229
return execute(
233230
'read_document',
234231
knowledgeOperations.readDocument,

‎apps/sim/lib/knowledge/search/filters.test.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,17 @@ import { liveCitationId } from '@/lib/knowledge/search/citation'
33
import { intersectWorkspaceSearchFilters } from '@/lib/knowledge/search/filters'
44
import { encodeLiveReference, liveReferenceOwner } from '@/lib/sim-search/live/reference'
55

6+
const owner = liveReferenceOwner('reader', 'workspace:w')
67
const fields = {
7-
owner: liveReferenceOwner('reader', 'workspace:w'),
88
account: 'account',
99
provider: 'slack' as const,
1010
id: '1730912345.123456',
1111
container: 'C1',
1212
}
1313
/** One Slack message in the current ID form and in the earlier form older conversations hold. */
14-
const currentId = encodeLiveReference(fields)
15-
const { owner: _owner, ...located } = fields
14+
const currentId = encodeLiveReference(fields, owner)
1615
const earlierId = `live:${Buffer.from(
17-
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...located })
16+
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...fields })
1817
).toString('base64url')}`
1918

2019
describe('Assistant search scope', () => {
@@ -59,7 +58,7 @@ describe('live citation IDs', () => {
5958
it('cite one document as one source whichever ID form named it', () => {
6059
expect(liveCitationId(earlierId)).toBe(liveCitationId(currentId))
6160
expect(liveCitationId(currentId)).not.toBe(
62-
liveCitationId(encodeLiveReference({ ...fields, id: '1730912345.123457' }))
61+
liveCitationId(encodeLiveReference({ ...fields, id: '1730912345.123457' }, owner))
6362
)
6463
})
6564
})

‎apps/sim/lib/sim-search/live/application.test.ts‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,14 +1192,10 @@ describe('authorized live retrieval', () => {
11921192
describe('live document IDs', () => {
11931193
const workspaceScope = resourceScopeKey(resourceScopeFromOwner({ workspaceId: 'workspace' }))
11941194
/** The earlier wire form, spelled out so these tests pin what older conversations hold. */
1195-
const legacyId = ({ owner: _owner, ...fields }: Record<string, string>) =>
1195+
const legacyId = (fields: Record<string, string>) =>
11961196
`live:${Buffer.from(JSON.stringify({ v: 1, ...fields })).toString('base64url')}`
1197-
const fields = {
1198-
owner: liveReferenceOwner('reader', workspaceScope),
1199-
account: 'account',
1200-
provider: 'google_drive' as const,
1201-
id: 'doc',
1202-
}
1197+
const readerOwner = liveReferenceOwner('reader', workspaceScope)
1198+
const fields = { account: 'account', provider: 'google_drive' as const, id: 'doc' }
12031199
const read = (documentId: string, extra: Record<string, unknown> = {}, as = principal) =>
12041200
readLiveDocument.execute({
12051201
principal: as,
@@ -1217,8 +1213,8 @@ describe('authorized live retrieval', () => {
12171213
const [first] = search.results
12181214
if (!first) throw new Error('Expected a search result')
12191215
const { documentId } = first
1220-
expect(documentId).toBe(encodeLiveReference(fields))
1221-
expect(documentId).toMatch(/^live2:google_drive~[0-9a-f]{8}~account~doc~[0-9a-f]{6}$/)
1216+
expect(documentId).toBe(encodeLiveReference(fields, readerOwner))
1217+
expect(documentId).toMatch(/^live2:google_drive~account~doc~[0-9a-f]{6}$/)
12221218
const result = await read(documentId)
12231219
expect(result.documentId).toBe(documentId)
12241220
expect(result.chunks[0]?.content).toBe('Evidence')
@@ -1235,7 +1231,7 @@ describe('authorized live retrieval', () => {
12351231
throw new NativeSearchError('reconnect', 'This search connection is no longer available.')
12361232
return { account, accessToken: 'secret' }
12371233
})
1238-
const theirs = encodeLiveReference({ ...fields, account: 'their-account' })
1234+
const theirs = encodeLiveReference({ ...fields, account: 'their-account' }, readerOwner)
12391235
await expect(read(theirs)).rejects.toMatchObject({ code: 'unauthorized' })
12401236
expect(mocks.resolveAccount).toHaveBeenCalledWith(
12411237
expect.anything(),
@@ -1255,7 +1251,7 @@ describe('authorized live retrieval', () => {
12551251
])(
12561252
'refuses an ID minted for %s as not found before resolving any account',
12571253
async (_case, user, scope) => {
1258-
const theirs = encodeLiveReference({ ...fields, owner: liveReferenceOwner(user, scope) })
1254+
const theirs = encodeLiveReference(fields, liveReferenceOwner(user, scope))
12591255
await expect(read(theirs)).rejects.toThrow('Document not found in this search scope')
12601256
expect(mocks.resolveAccount).not.toHaveBeenCalled()
12611257
expect(mocks.read).not.toHaveBeenCalled()
@@ -1289,7 +1285,7 @@ describe('authorized live retrieval', () => {
12891285
}
12901286
)
12911287
it('matches a selected document across the earlier and current forms', async () => {
1292-
const current = encodeLiveReference(fields)
1288+
const current = encodeLiveReference(fields, readerOwner)
12931289
const earlier = legacyId({ user: 'reader', scope: workspaceScope, ...fields })
12941290
await expect(read(current, { filters: { documentIds: [earlier] } })).resolves.toMatchObject({
12951291
documentId: current,
@@ -1300,7 +1296,7 @@ describe('authorized live retrieval', () => {
13001296
).toBe(true)
13011297
await expect(
13021298
read(current, {
1303-
filters: { documentIds: [encodeLiveReference({ ...fields, id: 'other' })] },
1299+
filters: { documentIds: [encodeLiveReference({ ...fields, id: 'other' }, readerOwner)] },
13041300
})
13051301
).rejects.toThrow('outside the selected search filters')
13061302
})

‎apps/sim/lib/sim-search/live/application.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ import { LIVE_SEARCH_PROVIDER_IDS } from '@/lib/sim-search/live/provider-catalog
7676
import { liveSearchGuidance } from '@/lib/sim-search/live/providers'
7777
import { LiveReadError } from '@/lib/sim-search/live/read-error'
7878
import {
79-
decodeLiveReference,
8079
encodeLiveReference,
8180
includesLiveDocument,
8281
liveDocumentKey,
8382
liveReferenceOwner,
83+
readLiveReference,
8484
} from '@/lib/sim-search/live/reference'
8585
import { queryRelevance } from '@/lib/sim-search/live/text-match'
8686
import type { LiveAccount, NativeDocument } from '@/lib/sim-search/live/types'
@@ -204,17 +204,19 @@ function candidateFor(
204204
): LiveCandidate {
205205
return {
206206
document,
207-
documentId: encodeLiveReference({
208-
owner,
209-
account: account.id,
210-
provider: account.provider,
211-
id: document.id,
212-
...(document.container ? { container: document.container } : {}),
213-
...(document.kind ? { kind: document.kind } : {}),
214-
...(document.revision ? { revision: document.revision } : {}),
215-
...(document.connectionBinding ? { connectionBinding: document.connectionBinding } : {}),
216-
...(document.threadId ? { threadId: document.threadId } : {}),
217-
}),
207+
documentId: encodeLiveReference(
208+
{
209+
account: account.id,
210+
provider: account.provider,
211+
id: document.id,
212+
...(document.container ? { container: document.container } : {}),
213+
...(document.kind ? { kind: document.kind } : {}),
214+
...(document.revision ? { revision: document.revision } : {}),
215+
...(document.connectionBinding ? { connectionBinding: document.connectionBinding } : {}),
216+
...(document.threadId ? { threadId: document.threadId } : {}),
217+
},
218+
owner
219+
),
218220
}
219221
}
220222

@@ -1039,12 +1041,10 @@ export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
10391041
if (input.organizationId) await requireOrganizationSearchAvailable(input.organizationId)
10401042
if (!Number.isInteger(input.limit) || input.limit < 1 || input.limit > 8)
10411043
throw new OrchestrationError('validation', 'Invalid document read limit')
1042-
const reference = decodeLiveReference(input.documentId)
1043-
if (
1044-
reference.owner !==
1044+
const reference = readLiveReference(
1045+
input.documentId,
10451046
liveReferenceOwner(userId, resourceScopeKey(resourceScopeFromOwner(input)))
10461047
)
1047-
throw new OrchestrationError('not_found', 'Document not found in this search scope')
10481048
if (
10491049
(input.filters?.source && input.filters.source !== reference.provider) ||
10501050
(input.filters?.documentIds &&
Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
import { describe, expect, it } from 'vitest'
22
import {
3-
decodeLiveReference,
43
encodeLiveReference,
54
type LiveReference,
5+
liveReferenceOwner,
6+
readLiveReference,
67
} from '@/lib/sim-search/live/reference'
78

8-
const owner = 'a1b2c3d4'
9+
const owner = liveReferenceOwner('reader', 'workspace:w')
910

1011
const thread: LiveReference = {
1112
provider: 'slack',
12-
owner,
1313
account: 'account',
1414
id: '1730912345.123456',
1515
container: 'C1',
1616
threadId: '1730900000.000100',
1717
}
1818

19-
const earlierForm = ({ owner: _owner, ...reference }: LiveReference) =>
19+
const earlierForm = (reference: LiveReference) =>
2020
`live:${Buffer.from(
2121
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...reference })
2222
).toString('base64url')}`
@@ -27,7 +27,21 @@ describe('live document IDs', () => {
2727
['dropping the thread it names', (id: string) => id.replace(/~1730900000\.000100(?=~)/, '')],
2828
['changing the message it names', (id: string) => id.replace('123456', '123457')],
2929
])('asks for a new search after %s rather than naming another document', (_case, damage) => {
30-
expect(() => decodeLiveReference(damage(encodeLiveReference(thread)))).toThrow('Search again')
30+
expect(() => readLiveReference(damage(encodeLiveReference(thread, owner)), owner)).toThrow(
31+
'Search again'
32+
)
33+
})
34+
35+
it.each([
36+
['another member', liveReferenceOwner('someone-else', 'workspace:w')],
37+
['another scope', liveReferenceOwner('reader', 'workspace:other')],
38+
])('refuses an ID read by %s as not found in this scope', (_case, reader) => {
39+
expect(() => readLiveReference(encodeLiveReference(thread, owner), reader)).toThrow(
40+
'Document not found in this search scope'
41+
)
42+
expect(() => readLiveReference(earlierForm(thread), reader)).toThrow(
43+
'Document not found in this search scope'
44+
)
3145
})
3246

3347
it.each([
@@ -36,17 +50,20 @@ describe('live document IDs', () => {
3650
['Unicode text', 'docs/設計/仕様書.md'],
3751
['an unpaired surrogate', 'name\ud800end'],
3852
])('reads back an ID holding %s', (_case, id) => {
39-
const reference = { provider: 'gitlab', owner, account: 'account', id } as const
40-
expect(decodeLiveReference(encodeLiveReference(reference))).toEqual(reference)
53+
const reference = { provider: 'gitlab', account: 'account', id } as const
54+
expect(readLiveReference(encodeLiveReference(reference, owner), owner)).toEqual(reference)
55+
})
56+
57+
it('reads an earlier-form ID minted for the same reader and scope', () => {
58+
expect(readLiveReference(earlierForm(thread), owner)).toEqual(thread)
4159
})
4260

4361
it('is never longer than the earlier form for the same document', () => {
4462
const reference = {
4563
provider: 'gitlab',
46-
owner,
4764
account: 'account',
4865
id: `src/${'設計書'.repeat(150)}.md`,
4966
} as const
50-
expect(encodeLiveReference(reference).length).toBeLessThan(earlierForm(reference).length)
67+
expect(encodeLiveReference(reference, owner).length).toBeLessThan(earlierForm(reference).length)
5168
})
5269
})

0 commit comments

Comments
 (0)