Skip to content

Commit fe1db96

Browse files
committed
fix(knowledge): validate upload processing options without stranding live sessions
recipe and lang were accepted as free strings up to their length caps, silently discarded, and echoed back nowhere, so a typo was unobservable: uploading with a misspelled recipe returned 200 and quietly used the default. Both are now validated at the boundary and a bad value answers 400 naming what is accepted. The accepted recipe set deliberately includes the sentinel every first-party caller sends today alongside the three real chunker recipes, and the three are derived from the chunker's own union so removing one there is a compile error here rather than a silent 400 in production. The same schema also parses metadata read back off a persisted upload session, so tightening it would have thrown out of resume and complete for any session created before this - a 500 on work that could then never finish. The read-back path now drops a value it no longer recognises instead of rejecting it; the request boundary stays strict. Neither field reaches chunking, so nothing here moves chunk boundaries, embeddings, or search results.
1 parent ebccc99 commit fe1db96

6 files changed

Lines changed: 221 additions & 15 deletions

File tree

apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { useQueryClient } from '@tanstack/react-query'
55
import type { V2KnowledgeDocumentSummary } from '@/lib/api/contracts/v2/knowledge'
6+
import type { KnowledgeDocumentUploadRecipe } from '@/lib/knowledge/upload-metadata'
67
import {
78
assertMultiFileUploadAdmission,
89
MultiFileUploadAdmissionError,
@@ -49,7 +50,7 @@ export interface UploadError {
4950
}
5051

5152
export interface ProcessingOptions {
52-
recipe?: string
53+
recipe?: KnowledgeDocumentUploadRecipe
5354
}
5455

5556
export interface UseKnowledgeUploadOptions {

apps/sim/lib/api/contracts/v2/knowledge.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ import {
5959
rerankerModelSchema,
6060
rerankerStatusSchema,
6161
} from '@/lib/knowledge/reranker-models'
62-
import { knowledgeDocumentUploadMetadataSchema } from '@/lib/knowledge/upload-metadata'
62+
import {
63+
KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES,
64+
knowledgeDocumentUploadMetadataSchema,
65+
} from '@/lib/knowledge/upload-metadata'
6366
import { MAX_KNOWLEDGE_DOCUMENT_FILE_SIZE } from '@/lib/uploads/shared/types'
6467

6568
/**
@@ -481,10 +484,14 @@ const v2KnowledgeDocumentProcessingOptionsSchema =
481484
.extend({
482485
recipe: knowledgeDocumentUploadMetadataSchema.shape.processingOptions
483486
.unwrap()
484-
.shape.recipe.describe('Optional document processing recipe.'),
487+
.shape.recipe.describe(
488+
`Optional document processing recipe. One of: ${KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES.join(', ')}.`
489+
),
485490
lang: knowledgeDocumentUploadMetadataSchema.shape.processingOptions
486491
.unwrap()
487-
.shape.lang.describe('Optional document language code.'),
492+
.shape.lang.describe(
493+
'Optional document language, as a BCP-47 tag such as `en` or `en-US`.'
494+
),
488495
})
489496
.strict()
490497

apps/sim/lib/knowledge/application/upload-sessions.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,46 @@ describe('knowledge-document upload application lifecycle', () => {
362362
)
363363
})
364364

365+
it('completes a session whose persisted recipe and lang predate their validation', async () => {
366+
mocks.getUpload.mockResolvedValue({
367+
...SESSION,
368+
metadata: {
369+
...SESSION.metadata,
370+
processingOptions: { recipe: 'super-chunker-9000', lang: 'en_US' },
371+
},
372+
})
373+
mocks.completeUpload.mockImplementation(
374+
async (params: {
375+
session: UploadSessionRecord
376+
finalize: (session: UploadSessionRecord) => Promise<{
377+
value: { document: typeof DOCUMENT; created: boolean; knowledgeBaseName: string | null }
378+
completedFileId?: string
379+
}>
380+
}) => {
381+
const finalized = await params.finalize(params.session)
382+
return {
383+
session: { ...params.session, status: 'completed' as const },
384+
value: finalized.value,
385+
alreadyCompleted: false,
386+
}
387+
}
388+
)
389+
390+
const result = await completeKnowledgeDocumentUpload.execute({
391+
principal: PRINCIPAL,
392+
input: {
393+
knowledgeBaseId: 'knowledge-1',
394+
assertedWorkspaceId: 'workspace-1',
395+
uploadId: 'upload-1',
396+
uploadToken: 'token',
397+
source: 'api',
398+
},
399+
request: REQUEST,
400+
})
401+
402+
expect(result.value.created).toBe(true)
403+
})
404+
365405
it('returns an already-bound document without re-billing, re-registering, or auditing', async () => {
366406
mocks.findBound.mockResolvedValue({ status: 'bound', document: DOCUMENT })
367407
mocks.completeUpload.mockImplementation(

apps/sim/lib/knowledge/application/upload-sessions.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type { CreatedKnowledgeDocument } from '@/lib/knowledge/orchestration/doc
2323
import { findBoundKnowledgeDocument } from '@/lib/knowledge/orchestration/documents'
2424
import {
2525
type KnowledgeDocumentUploadMetadata,
26-
knowledgeDocumentUploadMetadataSchema,
26+
persistedKnowledgeDocumentUploadMetadataSchema,
2727
} from '@/lib/knowledge/upload-metadata'
2828
import { recordKnowledgeBaseFileOwnership } from '@/lib/uploads/server/metadata'
2929
import { requestOrigin } from '@/lib/uploads/upload-session/application'
@@ -456,9 +456,13 @@ async function reauthorizeKnowledgeDocumentUpload(
456456
return context
457457
}
458458

459+
/**
460+
* Reads metadata back off a persisted session, so it uses the lenient schema:
461+
* a session created before `recipe`/`lang` were constrained must still resume.
462+
*/
459463
function knowledgeDocumentMetadataFor(session: UploadSessionRecord) {
460464
const { authBinding: _authBinding, ...metadata } = session.metadata
461-
return knowledgeDocumentUploadMetadataSchema.parse(metadata)
465+
return persistedKnowledgeDocumentUploadMetadataSchema.parse(metadata)
462466
}
463467

464468
function knowledgeDocumentInputFor(session: UploadSessionRecord) {
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES,
7+
knowledgeDocumentUploadMetadataSchema,
8+
persistedKnowledgeDocumentUploadMetadataSchema,
9+
} from '@/lib/knowledge/upload-metadata'
10+
11+
describe('knowledgeDocumentUploadMetadataSchema', () => {
12+
it('rejects a recipe outside the accepted set', () => {
13+
const result = knowledgeDocumentUploadMetadataSchema.safeParse({
14+
processingOptions: { recipe: 'totally-bogus-recipe' },
15+
})
16+
expect(result.success).toBe(false)
17+
expect(result.error?.issues[0]?.path).toEqual(['processingOptions', 'recipe'])
18+
expect(result.error?.issues[0]?.message).toContain('recipe must be one of')
19+
})
20+
21+
it('rejects a lang that is not a BCP-47 tag', () => {
22+
const result = knowledgeDocumentUploadMetadataSchema.safeParse({
23+
processingOptions: { lang: 'zzzz-nonsense!' },
24+
})
25+
expect(result.success).toBe(false)
26+
expect(result.error?.issues[0]?.message).toContain('BCP-47')
27+
})
28+
29+
it('rejects the underscore locale form callers reach for', () => {
30+
expect(
31+
knowledgeDocumentUploadMetadataSchema.safeParse({ processingOptions: { lang: 'en_US' } })
32+
.success
33+
).toBe(false)
34+
})
35+
36+
it('accepts what first-party callers actually send today', () => {
37+
const result = knowledgeDocumentUploadMetadataSchema.safeParse({
38+
tag1: 'product',
39+
processingOptions: { recipe: 'default', lang: 'en' },
40+
})
41+
expect(result.success).toBe(true)
42+
expect(result.data?.processingOptions).toEqual({ recipe: 'default', lang: 'en' })
43+
})
44+
45+
it('accepts a multi-subtag BCP-47 tag', () => {
46+
expect(
47+
knowledgeDocumentUploadMetadataSchema.safeParse({ processingOptions: { lang: 'zh-Hant-TW' } })
48+
.success
49+
).toBe(true)
50+
})
51+
52+
it('keeps the chunker recipes accepted alongside the default sentinel', () => {
53+
expect(KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES).toContain('default')
54+
expect(
55+
knowledgeDocumentUploadMetadataSchema.safeParse({ processingOptions: { recipe: 'markdown' } })
56+
.success
57+
).toBe(true)
58+
})
59+
})
60+
61+
describe('persistedKnowledgeDocumentUploadMetadataSchema', () => {
62+
it('drops a recipe persisted before the enum landed instead of throwing', () => {
63+
const parsed = persistedKnowledgeDocumentUploadMetadataSchema.parse({
64+
tag1: 'product',
65+
processingOptions: { recipe: 'super-chunker-9000', lang: 'en' },
66+
})
67+
expect(parsed.processingOptions).toEqual({ recipe: undefined, lang: 'en' })
68+
expect(parsed.tag1).toBe('product')
69+
})
70+
71+
it('drops a lang persisted before the BCP-47 shape landed instead of throwing', () => {
72+
const parsed = persistedKnowledgeDocumentUploadMetadataSchema.parse({
73+
processingOptions: { recipe: 'default', lang: 'en_US' },
74+
})
75+
expect(parsed.processingOptions).toEqual({ recipe: 'default', lang: undefined })
76+
})
77+
78+
it('does not throw on a session whose processing options are wholly unrecognized', () => {
79+
expect(() =>
80+
persistedKnowledgeDocumentUploadMetadataSchema.parse({
81+
processingOptions: { recipe: 42, lang: false },
82+
})
83+
).not.toThrow()
84+
})
85+
86+
it('preserves recognized values', () => {
87+
const parsed = persistedKnowledgeDocumentUploadMetadataSchema.parse({
88+
processingOptions: { recipe: 'code', lang: 'en-US' },
89+
})
90+
expect(parsed.processingOptions).toEqual({ recipe: 'code', lang: 'en-US' })
91+
})
92+
})
Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,90 @@
11
import { z } from 'zod'
2+
import type { RecursiveRecipe } from '@/lib/chunkers/types'
3+
4+
/**
5+
* Recipes the recursive chunker implements. Mirrors `RecursiveRecipe`; the
6+
* `satisfies` keeps a rename or removal there a compile error here.
7+
*/
8+
const RECURSIVE_RECIPES = [
9+
'plain',
10+
'markdown',
11+
'code',
12+
] as const satisfies readonly RecursiveRecipe[]
13+
14+
/**
15+
* Recipes accepted on a document upload. `'default'` is not a chunker recipe —
16+
* it is the long-standing sentinel every first-party caller sends to mean "use
17+
* the knowledge base's configured strategy", so it must stay accepted.
18+
*/
19+
export const KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES = ['default', ...RECURSIVE_RECIPES] as const
20+
21+
export type KnowledgeDocumentUploadRecipe = (typeof KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES)[number]
22+
23+
/**
24+
* BCP-47 language tag: a 2-8 letter primary subtag followed by any number of
25+
* alphanumeric subtags, e.g. `en`, `en-US`, `zh-Hant-TW`.
26+
*/
27+
const BCP47_LANGUAGE_TAG = /^[A-Za-z]{2,8}(?:-[A-Za-z0-9]{1,8})*$/
228

329
const knowledgeDocumentUploadTagSchema = z
430
.string()
531
.max(1000, 'Knowledge document tag values cannot exceed 1000 characters')
632
.optional()
733

34+
const knowledgeDocumentUploadTagShape = {
35+
tag1: knowledgeDocumentUploadTagSchema,
36+
tag2: knowledgeDocumentUploadTagSchema,
37+
tag3: knowledgeDocumentUploadTagSchema,
38+
tag4: knowledgeDocumentUploadTagSchema,
39+
tag5: knowledgeDocumentUploadTagSchema,
40+
tag6: knowledgeDocumentUploadTagSchema,
41+
tag7: knowledgeDocumentUploadTagSchema,
42+
}
43+
44+
const recipeSchema = z.enum(KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES, {
45+
error: `recipe must be one of: ${KNOWLEDGE_DOCUMENT_UPLOAD_RECIPES.join(', ')}`,
46+
})
47+
48+
const langSchema = z
49+
.string()
50+
.max(35, 'lang cannot exceed 35 characters')
51+
.regex(BCP47_LANGUAGE_TAG, 'lang must be a BCP-47 language tag, for example "en" or "en-US"')
52+
853
/** Persisted metadata stored with a resumable Knowledge document upload session. */
954
export const knowledgeDocumentUploadMetadataSchema = z
1055
.object({
11-
tag1: knowledgeDocumentUploadTagSchema,
12-
tag2: knowledgeDocumentUploadTagSchema,
13-
tag3: knowledgeDocumentUploadTagSchema,
14-
tag4: knowledgeDocumentUploadTagSchema,
15-
tag5: knowledgeDocumentUploadTagSchema,
16-
tag6: knowledgeDocumentUploadTagSchema,
17-
tag7: knowledgeDocumentUploadTagSchema,
56+
...knowledgeDocumentUploadTagShape,
1857
processingOptions: z
1958
.object({
20-
recipe: z.string().max(255, 'recipe cannot exceed 255 characters').optional(),
21-
lang: z.string().max(35, 'lang cannot exceed 35 characters').optional(),
59+
recipe: recipeSchema.optional(),
60+
lang: langSchema.optional(),
2261
})
2362
.strict()
2463
.optional(),
2564
})
2665
.strict()
2766

2867
export type KnowledgeDocumentUploadMetadata = z.output<typeof knowledgeDocumentUploadMetadataSchema>
68+
69+
/**
70+
* Read-back variant for metadata already persisted on an upload session.
71+
*
72+
* The strict schema above is a *request* boundary and rejects an unrecognized
73+
* `recipe`/`lang`. Sessions created before those constraints existed can carry
74+
* values that no longer parse, and rejecting them here would throw a raw
75+
* `ZodError` out of resume/complete — a 500 on a session that could never be
76+
* finished. Unrecognized values are dropped instead; neither field affects
77+
* processing today, so dropping one changes nothing but the analytics property.
78+
*/
79+
export const persistedKnowledgeDocumentUploadMetadataSchema = z
80+
.object({
81+
...knowledgeDocumentUploadTagShape,
82+
processingOptions: z
83+
.object({
84+
recipe: recipeSchema.optional().catch(undefined),
85+
lang: langSchema.optional().catch(undefined),
86+
})
87+
.strict()
88+
.optional(),
89+
})
90+
.strict()

0 commit comments

Comments
 (0)