Skip to content

Commit 293d58c

Browse files
committed
fix(search): refuse another reader's live ID as not found in this scope
1 parent 4021546 commit 293d58c

8 files changed

Lines changed: 98 additions & 50 deletions

File tree

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,19 @@ import { generateId } from '@sim/utils/id'
2525
import { isPlainRecord } from '@sim/utils/object'
2626
import { and, eq, inArray } from 'drizzle-orm'
2727
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
28+
import { resourceScopeFromOwner, resourceScopeKey } from '@/lib/core/resource-scope'
2829
import { seedKnowledgeAclFixture } from '@/lib/knowledge/__integration__/seed-source-access-fixture'
2930
import {
3031
createKnowledgeConnector,
3132
updateKnowledgeConnector,
3233
} from '@/lib/knowledge/application/connectors'
3334
import { refreshConnectorDirectory } from '@/lib/knowledge/connectors/external-group-sync'
3435
import { readLiveDocument, searchLiveKnowledge } from '@/lib/sim-search/live/application'
35-
import { decodeLiveReference } from '@/lib/sim-search/live/reference'
36+
import {
37+
decodeLiveReference,
38+
encodeLiveReference,
39+
liveReferenceOwner,
40+
} from '@/lib/sim-search/live/reference'
3641
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
3742

3843
interface FixturePerson {
@@ -128,6 +133,14 @@ describe.skipIf(!fixtureFile)('federated self-hosted GitLab Search', () => {
128133
})
129134
}
130135

136+
function documentForReader(documentId: string, person: FixturePerson) {
137+
const scope = resourceScopeKey(resourceScopeFromOwner({ organizationId: ids.organizationId }))
138+
return encodeLiveReference({
139+
...decodeLiveReference(documentId),
140+
owner: liveReferenceOwner(person.simId, scope),
141+
})
142+
}
143+
131144
beforeAll(async () => {
132145
const fixture: unknown = JSON.parse(await readFile(fixtureFile!, 'utf8'))
133146
if (
@@ -310,11 +323,13 @@ describe.skipIf(!fixtureFile)('federated self-hosted GitLab Search', () => {
310323
120_000
311324
)
312325

313-
it('checks a shared source reference against its reader and rechecks confidentiality', async () => {
326+
it('keeps references reader-bound and rechecks confidentiality before reads', async () => {
314327
const reporter = await search(people.reporter!, 'code')
315328
const file = reporter.results[0]!
316-
/** The ID names no reader: the guest's own GitLab access refuses the reporter's code result. */
317329
await expect(read(people.guest!, file.documentId)).rejects.toMatchObject({ code: 'not_found' })
330+
await expect(
331+
read(people.guest!, documentForReader(file.documentId, people.guest!))
332+
).rejects.toBeDefined()
318333
const guest = await search(people.guest!, 'issues')
319334
const normal = guest.results.find(
320335
(row) => decodeLiveReference(row.documentId).id === String(issueIid)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ 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 } from '@/lib/sim-search/live/reference'
58+
import { encodeLiveReference, liveReferenceOwner } from '@/lib/sim-search/live/reference'
5959
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
6060

6161
const mocks = hoisted
@@ -311,7 +311,11 @@ describe('batched document reads', () => {
311311
const earlier = `live:${Buffer.from(
312312
JSON.stringify({ v: 1, user: 'person-1', scope: 'organization:org-1', ...reference })
313313
).toString('base64url')}`
314-
await call('read_document', { documentIds: [earlier, encodeLiveReference(reference)] })
314+
const current = encodeLiveReference({
315+
...reference,
316+
owner: liveReferenceOwner('person-1', 'organization:org-1'),
317+
})
318+
await call('read_document', { documentIds: [earlier, current] })
315319
expect(mocks.rateLimit).toHaveBeenCalledWith(request, auth, expect.anything(), 1)
316320
expect(mocks.liveRead).toHaveBeenCalledTimes(1)
317321
})

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
11
import { describe, expect, it } from 'vitest'
22
import { liveCitationId } from '@/lib/knowledge/search/citation'
33
import { intersectWorkspaceSearchFilters } from '@/lib/knowledge/search/filters'
4-
import { encodeLiveReference } from '@/lib/sim-search/live/reference'
4+
import { encodeLiveReference, liveReferenceOwner } from '@/lib/sim-search/live/reference'
55

66
const fields = {
7+
owner: liveReferenceOwner('reader', 'workspace:w'),
78
account: 'account',
89
provider: 'slack' as const,
910
id: '1730912345.123456',
1011
container: 'C1',
1112
}
1213
/** One Slack message in the current ID form and in the earlier form older conversations hold. */
1314
const currentId = encodeLiveReference(fields)
15+
const { owner: _owner, ...located } = fields
1416
const earlierId = `live:${Buffer.from(
15-
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...fields })
17+
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...located })
1618
).toString('base64url')}`
1719

1820
describe('Assistant search scope', () => {

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

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,11 @@ import { NativeSearchError } from '@/lib/sim-search/live/http'
8888
import { createPolicyVerifier } from '@/lib/sim-search/live/policy'
8989
import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema'
9090
import { livePolicyFor } from '@/lib/sim-search/live/policy-store'
91-
import { decodeLiveReference, encodeLiveReference } from '@/lib/sim-search/live/reference'
91+
import {
92+
decodeLiveReference,
93+
encodeLiveReference,
94+
liveReferenceOwner,
95+
} from '@/lib/sim-search/live/reference'
9296
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
9397

9498
workspaceAuthzMockFns.mockPermissionSatisfies.mockImplementation(
@@ -1188,9 +1192,14 @@ describe('authorized live retrieval', () => {
11881192
describe('live document IDs', () => {
11891193
const workspaceScope = resourceScopeKey(resourceScopeFromOwner({ workspaceId: 'workspace' }))
11901194
/** The earlier wire form, spelled out so these tests pin what older conversations hold. */
1191-
const legacyId = (fields: Record<string, string>) =>
1195+
const legacyId = ({ owner: _owner, ...fields }: Record<string, string>) =>
11921196
`live:${Buffer.from(JSON.stringify({ v: 1, ...fields })).toString('base64url')}`
1193-
const fields = { account: 'account', provider: 'google_drive' as const, id: 'doc' }
1197+
const fields = {
1198+
owner: liveReferenceOwner('reader', workspaceScope),
1199+
account: 'account',
1200+
provider: 'google_drive' as const,
1201+
id: 'doc',
1202+
}
11941203
const read = (documentId: string, extra: Record<string, unknown> = {}, as = principal) =>
11951204
readLiveDocument.execute({
11961205
principal: as,
@@ -1205,8 +1214,11 @@ describe('authorized live retrieval', () => {
12051214

12061215
it('mints a compact ID naming no reader or scope that reads back', async () => {
12071216
const search = await searchLiveKnowledge.execute({ principal, input })
1208-
const documentId = search.results[0]!.documentId
1209-
expect(documentId).toMatch(/^live2:google_drive~account~doc~[0-9a-f]{6}$/)
1217+
const [first] = search.results
1218+
if (!first) throw new Error('Expected a search result')
1219+
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}$/)
12101222
const result = await read(documentId)
12111223
expect(result.documentId).toBe(documentId)
12121224
expect(result.chunks[0]?.content).toBe('Evidence')
@@ -1233,19 +1245,22 @@ describe('authorized live retrieval', () => {
12331245
)
12341246
expect(mocks.read).not.toHaveBeenCalled()
12351247
})
1236-
it("gates a shared account's ID by the reader's own verification", async () => {
1237-
const search = await searchLiveKnowledge.execute({ principal, input })
1238-
mocks.service.mockResolvedValue({
1239-
policy: defaultLiveSearchPolicy(),
1240-
partial: false,
1241-
verify: async () => false,
1242-
})
1243-
const other = createSessionPrincipal({ userId: 'other-reader', sessionId: 's2' })
1244-
await expect(read(search.results[0]!.documentId, {}, other)).rejects.toThrow(
1245-
'outside your organization'
1246-
)
1247-
expect(mocks.read).not.toHaveBeenCalled()
1248-
})
1248+
it.each([
1249+
['another member', 'other-reader', workspaceScope],
1250+
[
1251+
'another scope',
1252+
'reader',
1253+
resourceScopeKey(resourceScopeFromOwner({ workspaceId: 'other' })),
1254+
],
1255+
])(
1256+
'refuses an ID minted for %s as not found before resolving any account',
1257+
async (_case, user, scope) => {
1258+
const theirs = encodeLiveReference({ ...fields, owner: liveReferenceOwner(user, scope) })
1259+
await expect(read(theirs)).rejects.toThrow('Document not found in this search scope')
1260+
expect(mocks.resolveAccount).not.toHaveBeenCalled()
1261+
expect(mocks.read).not.toHaveBeenCalled()
1262+
}
1263+
)
12491264
it.each([
12501265
['a missing field', 'live2:google_drive~account'],
12511266
['an unknown provider', 'live2:nowhere~account~doc'],

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import {
8080
encodeLiveReference,
8181
includesLiveDocument,
8282
liveDocumentKey,
83+
liveReferenceOwner,
8384
} from '@/lib/sim-search/live/reference'
8485
import { queryRelevance } from '@/lib/sim-search/live/text-match'
8586
import type { LiveAccount, NativeDocument } from '@/lib/sim-search/live/types'
@@ -196,10 +197,15 @@ interface LiveCandidate {
196197
documentId: string
197198
}
198199

199-
function candidateFor(document: NativeDocument, account: LiveAccount): LiveCandidate {
200+
function candidateFor(
201+
document: NativeDocument,
202+
account: LiveAccount,
203+
owner: string
204+
): LiveCandidate {
200205
return {
201206
document,
202207
documentId: encodeLiveReference({
208+
owner,
203209
account: account.id,
204210
provider: account.provider,
205211
id: document.id,
@@ -431,6 +437,7 @@ const liveKnowledgeSearch = defineAuthorizedKnowledgeUseCase({
431437
const userId = requirePrincipalSubjectUserId(principal)
432438
if (input.organizationId) await requireOrganizationSearchAvailable(input.organizationId)
433439
input.signal?.throwIfAborted()
440+
const owner = liveReferenceOwner(userId, resourceScopeKey(resourceScopeFromOwner(input)))
434441
const queries = input.nativeQueries
435442
? nativeSearchQueriesSchema.parse(input.nativeQueries)
436443
: undefined
@@ -569,7 +576,7 @@ const liveKnowledgeSearch = defineAuthorizedKnowledgeUseCase({
569576
)
570577
const candidates = page.documents
571578
.filter((document) => document.id)
572-
.map((document) => candidateFor(document, account))
579+
.map((document) => candidateFor(document, account, owner))
573580
const matchingOf = (verified: LiveCandidate[]) =>
574581
firstOfEachDocument(
575582
verified.filter(({ document, documentId }) =>
@@ -1032,11 +1039,10 @@ export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
10321039
if (input.organizationId) await requireOrganizationSearchAvailable(input.organizationId)
10331040
if (!Number.isInteger(input.limit) || input.limit < 1 || input.limit > 8)
10341041
throw new OrchestrationError('validation', 'Invalid document read limit')
1035-
const { legacy, ...reference } = decodeLiveReference(input.documentId)
1036-
// A v2 ID names no reader or scope: the account resolution below decides both.
1042+
const reference = decodeLiveReference(input.documentId)
10371043
if (
1038-
legacy &&
1039-
(legacy.user !== userId || legacy.scope !== resourceScopeKey(resourceScopeFromOwner(input)))
1044+
reference.owner !==
1045+
liveReferenceOwner(userId, resourceScopeKey(resourceScopeFromOwner(input)))
10401046
)
10411047
throw new OrchestrationError('not_found', 'Document not found in this search scope')
10421048
if (

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@ import {
55
type LiveReference,
66
} from '@/lib/sim-search/live/reference'
77

8+
const owner = 'a1b2c3d4'
9+
810
const thread: LiveReference = {
911
provider: 'slack',
12+
owner,
1013
account: 'account',
1114
id: '1730912345.123456',
1215
container: 'C1',
1316
threadId: '1730900000.000100',
1417
}
1518

16-
const earlierForm = (reference: LiveReference) =>
19+
const earlierForm = ({ owner: _owner, ...reference }: LiveReference) =>
1720
`live:${Buffer.from(
1821
JSON.stringify({ v: 1, user: 'reader', scope: 'workspace:w', ...reference })
1922
).toString('base64url')}`
@@ -33,13 +36,14 @@ describe('live document IDs', () => {
3336
['Unicode text', 'docs/設計/仕様書.md'],
3437
['an unpaired surrogate', 'name\ud800end'],
3538
])('reads back an ID holding %s', (_case, id) => {
36-
const reference = { provider: 'gitlab', account: 'account', id } as const
39+
const reference = { provider: 'gitlab', owner, account: 'account', id } as const
3740
expect(decodeLiveReference(encodeLiveReference(reference))).toEqual(reference)
3841
})
3942

4043
it('is never longer than the earlier form for the same document', () => {
4144
const reference = {
4245
provider: 'gitlab',
46+
owner,
4347
account: 'account',
4448
id: `src/${'設計書'.repeat(150)}.md`,
4549
} as const

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

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,28 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
88
*
99
* They are not an authorization boundary: every read re-resolves the account among the reader's
1010
* own accounts in the request's scope, then re-verifies the document with the provider and the
11-
* admin's source policy. So the ID carries only what locates the document, and nothing the
12-
* request already knows (the reader, the scope).
11+
* admin's source policy. An ID names its reader and scope only by a short {@link liveReferenceOwner}
12+
* fingerprint, so a read in another scope or by another member is refused as not found in this
13+
* scope rather than reaching account resolution.
1314
*
1415
* The current form is `live2:` followed by `~`-separated fields, trailing empty fields dropped,
15-
* then a checksum: `live2:slack~<account>~<ts>~<channel>~~<thread ts>~<checksum>`. Clients copy
16-
* IDs verbatim into later calls, so the form stays readable (provider IDs and UUIDs tokenize far
17-
* better than base64 of them) and uses only characters a model reproduces reliably. Earlier IDs
18-
* (`live:` + base64url JSON naming the reader and scope) still read, with their reader and scope
19-
* checked as before.
16+
* then a checksum: `live2:slack~<owner>~<account>~<ts>~<channel>~~<thread ts>~<checksum>`.
17+
* Clients copy IDs verbatim into later calls, so the form stays readable (provider IDs and UUIDs
18+
* tokenize far better than base64 of them) and uses only characters a model reproduces reliably.
19+
* Earlier IDs (`live:` + base64url JSON naming the reader and scope) still read, checked against
20+
* the same fingerprint.
2021
*/
2122
const LIVE_V1_PREFIX = 'live:'
2223
const LIVE_V2_PREFIX = 'live2:'
2324
const MAX_REFERENCE_LENGTH = 4000
2425
const FIELD_SEPARATOR = '~'
2526
const CHECKSUM_LENGTH = 6
27+
const OWNER_LENGTH = 8
2628

2729
const fieldsSchema = z
2830
.object({
2931
provider: liveSearchProviderSchema,
32+
owner: z.string().regex(new RegExp(`^[0-9a-f]{${OWNER_LENGTH}}$`)),
3033
account: z.string().min(1).max(500),
3134
id: z.string().min(1).max(1000),
3235
container: z.string().min(1).max(500).optional(),
@@ -43,6 +46,7 @@ const fieldsSchema = z
4346
/** Field order of the v2 form; optional fields last so absent ones cost nothing. */
4447
const FIELD_ORDER = [
4548
'provider',
49+
'owner',
4650
'account',
4751
'id',
4852
'container',
@@ -73,9 +77,9 @@ const legacySchema = z
7377

7478
export type LiveReference = z.output<typeof fieldsSchema>
7579

76-
/** A decoded ID; `legacy` holds the reader and scope a v1 ID was minted for. */
77-
export type DecodedLiveReference = LiveReference & {
78-
legacy?: { user: string; scope: string }
80+
/** The reader and scope an ID was minted for, as the short fingerprint the ID carries. */
81+
export function liveReferenceOwner(userId: string, scope: string): string {
82+
return sha256Hex(`${userId}\u0000${scope}`).slice(0, OWNER_LENGTH)
7983
}
8084

8185
/**
@@ -131,7 +135,7 @@ function decodeV2(value: string): LiveReference {
131135
if (separator < 0 || value.slice(separator + 1) !== checksum(body))
132136
throw new Error('Checksum mismatch')
133137
const parts = body.split(FIELD_SEPARATOR)
134-
if (parts.length < 3 || parts.length > FIELD_ORDER.length) throw new Error('Invalid field count')
138+
if (parts.length < 4 || parts.length > FIELD_ORDER.length) throw new Error('Invalid field count')
135139
const fields: Record<string, string> = {}
136140
FIELD_ORDER.forEach((name, index) => {
137141
const part = parts[index]
@@ -140,20 +144,20 @@ function decodeV2(value: string): LiveReference {
140144
return fieldsSchema.parse(fields)
141145
}
142146

143-
function decodeV1(body: string): DecodedLiveReference {
147+
function decodeV1(body: string): LiveReference {
144148
const { v, user, scope, ...rest } = legacySchema.parse(
145149
JSON.parse(Buffer.from(body, 'base64url').toString('utf8'))
146150
)
147151
const fields = Object.fromEntries(
148152
Object.entries(rest).filter(([, value]) => value !== undefined && value !== '')
149153
)
150-
return { ...fieldsSchema.parse(fields), legacy: { user, scope } }
154+
return fieldsSchema.parse({ ...fields, owner: liveReferenceOwner(user, scope) })
151155
}
152156

153157
const SEARCH_AGAIN = 'Invalid live document reference. Search again to select a live document'
154158

155159
/** Decodes a current or earlier live document ID; anything else asks the caller to search again. */
156-
export function decodeLiveReference(value: string): DecodedLiveReference {
160+
export function decodeLiveReference(value: string): LiveReference {
157161
if (value.length > MAX_REFERENCE_LENGTH) throw new OrchestrationError('not_found', SEARCH_AGAIN)
158162
try {
159163
if (value.startsWith(LIVE_V2_PREFIX)) return decodeV2(value.slice(LIVE_V2_PREFIX.length))
@@ -172,8 +176,7 @@ export function liveDocumentKey(documentId: string): string {
172176
if (!documentId.startsWith(LIVE_V1_PREFIX) && !documentId.startsWith(LIVE_V2_PREFIX))
173177
return documentId
174178
try {
175-
const { legacy, ...reference } = decodeLiveReference(documentId)
176-
return encodeLiveReference(reference)
179+
return encodeLiveReference(decodeLiveReference(documentId))
177180
} catch {
178181
return documentId
179182
}

‎scripts/check-explicit-any.baseline.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,6 @@
15111511
"apps/sim/lib/knowledge/read-access.test.ts": 1,
15121512
"apps/sim/lib/knowledge/read-access.ts": 1,
15131513
"apps/sim/lib/knowledge/search/budget.test.ts": 1,
1514-
"apps/sim/lib/knowledge/search/filters.ts": 1,
15151514
"apps/sim/lib/knowledge/search/queries.test.ts": 10,
15161515
"apps/sim/lib/knowledge/search/queries.ts": 3,
15171516
"apps/sim/lib/knowledge/tags/filter-resolution.ts": 1,

0 commit comments

Comments
 (0)