Skip to content

Commit 39fe2b8

Browse files
committed
fix(search): protect query output and validate document continuation
1 parent 507de5a commit 39fe2b8

7 files changed

Lines changed: 78 additions & 20 deletions

File tree

apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,37 @@
11
import { ChipLink } from '@sim/emcn'
22
import { notFound, redirect } from 'next/navigation'
3+
import type { SearchParams } from 'nuqs/server'
34
import { readSearchDocumentResultSchema } from '@/lib/api/contracts/knowledge/documents'
45
import { getSession } from '@/lib/auth'
56
import { OrchestrationError } from '@/lib/core/orchestration/types'
67
import { readSearchDocument } from '@/lib/knowledge/application/read-search-document'
78
import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
9+
import {
10+
loadDocumentReadParams,
11+
serializeDocumentReadParams,
12+
} from '@/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params'
813
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
914
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
1015

1116
interface OrganizationDocumentPageProps {
1217
params: Promise<{ organizationId: string; knowledgeBaseId: string; documentId: string }>
13-
searchParams: Promise<{ offset?: string }>
18+
searchParams: Promise<SearchParams>
1419
}
1520

1621
export default async function OrganizationDocumentPage({
1722
params,
1823
searchParams,
1924
}: OrganizationDocumentPageProps) {
2025
const { organizationId, knowledgeBaseId, documentId } = await params
21-
const { offset: rawOffset } = await searchParams
22-
const offset = rawOffset === undefined ? 0 : Number(rawOffset)
23-
if (!Number.isInteger(offset) || offset < 0 || offset > 5000) notFound()
26+
const position = await loadDocumentReadParams(searchParams, { strict: true }).catch(() =>
27+
notFound()
28+
)
2429
const href = `/o/${encodeURIComponent(organizationId)}/knowledge/${encodeURIComponent(knowledgeBaseId)}/${encodeURIComponent(documentId)}`
2530
const session = await getSession()
2631
if (!session?.user) {
2732
redirect(
2833
buildAuthCrossLink('/login', {
29-
callbackUrl: offset ? `${href}?offset=${offset}` : href,
34+
callbackUrl: serializeDocumentReadParams(href, position),
3035
isInviteFlow: false,
3136
})
3237
)
@@ -39,15 +44,15 @@ export default async function OrganizationDocumentPage({
3944
input: {
4045
documentId,
4146
assertedOrganizationId: organizationId,
42-
offset,
43-
limit: 20,
47+
...position,
48+
limit: 3,
4449
resultSecretRegistry: registry,
4550
},
4651
})
4752
} catch (error) {
4853
if (
4954
error instanceof OrchestrationError &&
50-
(error.code === 'not_found' || error.code === 'forbidden')
55+
(error.code === 'not_found' || error.code === 'forbidden' || error.code === 'validation')
5156
)
5257
notFound()
5358
throw error
@@ -71,11 +76,11 @@ export default async function OrganizationDocumentPage({
7176
</p>
7277
))}
7378
<nav className='flex gap-2' aria-label='Document pages'>
74-
{offset > 0 && (
75-
<ChipLink href={`${href}?offset=${Math.max(0, offset - 20)}`}>Previous</ChipLink>
79+
{(position.startChunkIndex > 0 || position.startOffset > 0) && (
80+
<ChipLink href={href}>Start</ChipLink>
7681
)}
77-
{document.nextOffset !== null && (
78-
<ChipLink href={`${href}?offset=${document.nextOffset}`}>Next</ChipLink>
82+
{document.next && (
83+
<ChipLink href={serializeDocumentReadParams(href, document.next)}>Next</ChipLink>
7984
)}
8085
</nav>
8186
</article>
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { createLoader, createParser, createSerializer } from 'nuqs/server'
2+
3+
const parseAsDocumentPosition = createParser({
4+
parse: (value) => {
5+
if (!/^\d+$/.test(value)) return null
6+
const position = Number(value)
7+
return Number.isSafeInteger(position) && position <= 2147483647 ? position : null
8+
},
9+
serialize: String,
10+
}).withDefault(0)
11+
12+
export const documentReadParams = {
13+
startChunkIndex: parseAsDocumentPosition,
14+
startOffset: parseAsDocumentPosition,
15+
}
16+
17+
const documentReadUrlKeys = {
18+
urlKeys: {
19+
startChunkIndex: 'start-chunk-index',
20+
startOffset: 'start-offset',
21+
},
22+
} as const
23+
24+
export const loadDocumentReadParams = createLoader(documentReadParams, documentReadUrlKeys)
25+
export const serializeDocumentReadParams = createSerializer(documentReadParams, documentReadUrlKeys)

apps/sim/lib/copilot/tools/server/knowledge/workspace-search.test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ vi.mock('@/lib/knowledge/application/read-search-document', () => ({
3535
execute: mocks.read,
3636
},
3737
}))
38-
vi.mock('@/executor/utils/resolved-secret-content-projection', () => ({
39-
projectResolvedSecretModelContent: (value: unknown) => ({ safe: true, value }),
40-
}))
4138

4239
import {
4340
readDocumentServerTool,
@@ -188,6 +185,23 @@ describe('Assistant retrieval tools', () => {
188185
})
189186
)
190187
})
188+
it('returns only the projected query to the model', async () => {
189+
const secret = 'private-resolved-query-token'
190+
const registry = new ResolvedSecretTraceRegistry([
191+
{ name: 'TOKEN', plaintext: secret, encryptedValue: 'ciphertext' },
192+
])
193+
registry.recordResolved('TOKEN', secret)
194+
const result = await searchWorkspaceServerTool.execute(
195+
{ query: `Find ${secret}` },
196+
{ ...context, resolvedSecretTraceRegistry: registry }
197+
)
198+
expect(result).toMatchObject({ success: true, data: { query: 'Find {{TOKEN}}' } })
199+
expect(mocks.search).toHaveBeenCalledWith(
200+
expect.objectContaining({ input: expect.objectContaining({ query: 'Find {{TOKEN}}' }) })
201+
)
202+
expect(JSON.stringify(result)).not.toContain(secret)
203+
})
204+
191205
it.each([0, 20, 50])(
192206
'measures UTF-8 bytes for %i passages without logging their content',
193207
async (count) => {

apps/sim/lib/copilot/tools/server/knowledge/workspace-search.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const searchWorkspaceServerTool: BaseServerTool = {
9595
success: true,
9696
message: `${result.retrieval.status === 'partial' ? 'Partial search: a retrieval branch reached its deadline. These results cannot establish absence or completeness. ' : ''}Found ${result.results.length} passage previews. Read a document at its chunkIndex for more context. ${CITATION_INSTRUCTION}`,
9797
data: {
98-
query,
98+
query: safeQuery,
9999
retrieval: result.retrieval,
100100
results: result.results.map((item) => {
101101
const content = projectResolvedSecretModelContent(item.content, registry)

apps/sim/lib/knowledge/application/read-search-document.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,17 @@ describe('precise bounded passage expansion', () => {
253253
)
254254
})
255255

256+
it('rejects a continuation when all remaining chunks have disappeared', async () => {
257+
mocks.chunks.mockResolvedValue({
258+
chunks: [],
259+
pagination: { total: 2, hasMore: false },
260+
})
261+
await expect(
262+
readSearchDocument.execute({ principal, input: { ...input, startChunkIndex: 7 } })
263+
).rejects.toThrow('no longer available')
264+
expect(mocks.provenance).not.toHaveBeenCalled()
265+
})
266+
256267
it('rejects positions without an anchor and stale within-chunk continuation', async () => {
257268
await expect(
258269
readSearchDocument.execute({ principal, input: { ...input, startOffset: 2 } })

apps/sim/lib/knowledge/application/read-search-document.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ export interface ReadSearchDocumentInput {
3131
/** Bounds model text to at most 24KB of UTF-8, with continuation even inside a large chunk. */
3232
const READ_PAGE_CHARACTERS = 8000
3333
const READ_PAGE_CHUNKS = 8
34+
const STALE_POSITION_MESSAGE =
35+
'The passage position is no longer available; search again or read from the chunk start'
3436

3537
/** Reads enabled indexed passages with the same document scope and ACLs as search. */
3638
export const readSearchDocument = defineAuthorizedKnowledgeUseCase({
@@ -100,6 +102,9 @@ export const readSearchDocument = defineAuthorizedKnowledgeUseCase({
100102
)
101103
)
102104
if (page.pagination.total === 0) throw new OrchestrationError('not_found', 'Document not found')
105+
if (input.startChunkIndex !== undefined && page.chunks.length === 0) {
106+
throw new OrchestrationError('validation', STALE_POSITION_MESSAGE)
107+
}
103108
const provenance = await measureSearchStage('result_provenance', () =>
104109
importKnowledgeSearchResultSecretProvenance({
105110
registry: input.resultSecretRegistry,
@@ -131,10 +136,7 @@ export const readSearchDocument = defineAuthorizedKnowledgeUseCase({
131136
(projectedChunks[0]?.chunkIndex !== input.startChunkIndex ||
132137
input.startOffset >= projectedChunks[0].content.length)
133138
) {
134-
throw new OrchestrationError(
135-
'validation',
136-
'The passage position is no longer available; search again or read from the chunk start'
137-
)
139+
throw new OrchestrationError('validation', STALE_POSITION_MESSAGE)
138140
}
139141
let remaining = READ_PAGE_CHARACTERS
140142
const chunks: ReadSearchDocumentResult['chunks'] = []

apps/sim/lib/knowledge/application/workspace-search.activity.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ describe.each([
6363
queueTableRows(member, [{ role: 'member' }])
6464
expect(await operation.execute({ principal, input })).toEqual({
6565
results: [],
66+
retrieval: { status: 'complete', timedOutLegs: [] },
6667
query: 'policy',
6768
knowledgeBases: [],
6869
})

0 commit comments

Comments
 (0)