Skip to content

Commit 86ac309

Browse files
authored
fix(knowledge): validate tag slots and share the tag-name length limit (#6448)
Tag slots reached the DB unvalidated: the contract types them as plain strings, the slot column is `text` (its Drizzle `enum` is types-only), and the service casts before inserting. The create route now checks the slot against its declared field type via the existing isValidSlotForFieldType, and the bulk document route checks slot validity only, so renaming a definition that already stores a mismatched pair still works. The tag display name was capped at 100 on the bulk document route and unbounded everywhere else. Promote that number to a shared constant and apply it on every write path: both contracts, both modals, and the copilot create/update tools, whose names are model-generated and bypassed the contract entirely. Also drop two local copies of FIELD_TYPE_LABELS in favour of the shared one that two other components already import.
1 parent 585541a commit 86ac309

8 files changed

Lines changed: 133 additions & 24 deletions

File tree

apps/sim/app/api/knowledge/[id]/documents/[documentId]/tag-definitions/route.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { saveDocumentTagDefinitionsContract } from '@/lib/api/contracts/knowledg
55
import { parseRequest } from '@/lib/api/server'
66
import { getSession } from '@/lib/auth'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8-
import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
8+
import { getFieldTypeForSlot, SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
99
import {
1010
cleanupUnusedTagDefinitions,
1111
createOrUpdateTagDefinitionsBulk,
@@ -114,6 +114,16 @@ export const POST = withRouteHandler(
114114
{ status: 400 }
115115
)
116116
}
117+
/**
118+
* Slot validity only, not slot/field-type agreement: this route also renames
119+
* existing definitions, which resend whatever pair is already stored.
120+
*/
121+
if (getFieldTypeForSlot(def.tagSlot) === null) {
122+
return NextResponse.json(
123+
{ error: 'Invalid request data', details: `Unsupported tag slot: ${def.tagSlot}` },
124+
{ status: 400 }
125+
)
126+
}
117127
}
118128

119129
const bulkData: BulkTagDefinitionsData = {

apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ vi.mock('@/lib/knowledge/tags/service', () => ({
2323

2424
vi.mock('@/app/api/knowledge/utils', () => knowledgeApiUtilsMock)
2525

26+
import { KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH } from '@/lib/knowledge/constants'
2627
import { GET, POST } from '@/app/api/knowledge/[id]/tag-definitions/route'
2728

2829
const KB_ID = 'kb-victim'
@@ -140,5 +141,64 @@ describe('Knowledge Base Tag Definitions API Route', () => {
140141
expect(mockCheckKnowledgeBaseWriteAccess).not.toHaveBeenCalled()
141142
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
142143
})
144+
145+
it('rejects a tag slot this schema has no column for', async () => {
146+
authenticateAs('user-1', 'session')
147+
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)
148+
149+
const response = await POST(
150+
createMockRequest('POST', { ...CREATE_BODY, tagSlot: 'tag99' }),
151+
params()
152+
)
153+
154+
expect(response.status).toBe(400)
155+
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
156+
})
157+
158+
it('rejects a slot that belongs to a different field type', async () => {
159+
authenticateAs('user-1', 'session')
160+
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)
161+
162+
const response = await POST(
163+
createMockRequest('POST', {
164+
tagSlot: 'number1',
165+
displayName: 'Mismatch',
166+
fieldType: 'text',
167+
}),
168+
params()
169+
)
170+
171+
expect(response.status).toBe(400)
172+
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
173+
})
174+
175+
it('rejects an unsupported field type', async () => {
176+
authenticateAs('user-1', 'session')
177+
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)
178+
179+
const response = await POST(
180+
createMockRequest('POST', { ...CREATE_BODY, fieldType: 'nonsense' }),
181+
params()
182+
)
183+
184+
expect(response.status).toBe(400)
185+
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
186+
})
187+
188+
it('rejects a display name longer than the shared limit', async () => {
189+
authenticateAs('user-1', 'session')
190+
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)
191+
192+
const response = await POST(
193+
createMockRequest('POST', {
194+
...CREATE_BODY,
195+
displayName: 'a'.repeat(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH + 1),
196+
}),
197+
params()
198+
)
199+
200+
expect(response.status).toBe(400)
201+
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
202+
})
143203
})
144204
})

apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { createTagDefinitionContract } from '@/lib/api/contracts/knowledge'
55
import { parseRequest } from '@/lib/api/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8-
import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
8+
import { isValidSlotForFieldType } from '@/lib/knowledge/constants'
99
import { createTagDefinition, getTagDefinitions } from '@/lib/knowledge/tags/service'
1010
import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils'
1111

@@ -76,9 +76,18 @@ export const POST = withRouteHandler(
7676
if (!parsed.success) return parsed.response
7777

7878
const validatedData = parsed.data.body
79-
if (!(SUPPORTED_FIELD_TYPES as readonly string[]).includes(validatedData.fieldType)) {
79+
/**
80+
* The contract types `tagSlot` and `fieldType` as plain strings because
81+
* tightening them to enums cascades into UI form state types, so the pair is
82+
* checked here. Nothing downstream enforces it: the slot column is `text`
83+
* (its Drizzle `enum` is types-only) and the service casts before inserting.
84+
*/
85+
if (!isValidSlotForFieldType(validatedData.tagSlot, validatedData.fieldType)) {
8086
return NextResponse.json(
81-
{ error: 'Invalid request data', details: 'Invalid field type' },
87+
{
88+
error: 'Invalid request data',
89+
details: `Tag slot "${validatedData.tagSlot}" is not valid for field type "${validatedData.fieldType}"`,
90+
},
8291
{ status: 400 }
8392
)
8493
}

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@ import {
1818
} from '@sim/emcn'
1919
import { createLogger } from '@sim/logger'
2020
import { formatDate } from '@sim/utils/formatting'
21-
import { ALL_TAG_SLOTS, type AllTagSlot, MAX_TAG_SLOTS } from '@/lib/knowledge/constants'
21+
import {
22+
ALL_TAG_SLOTS,
23+
type AllTagSlot,
24+
FIELD_TYPE_LABELS,
25+
KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH,
26+
MAX_TAG_SLOTS,
27+
} from '@/lib/knowledge/constants'
2228
import type { DocumentTag } from '@/lib/knowledge/tags/types'
2329
import type { DocumentData } from '@/lib/knowledge/types'
2430
import {
@@ -30,14 +36,6 @@ import { useNextAvailableSlotMutation, useUpdateDocumentTags } from '@/hooks/que
3036

3137
const logger = createLogger('DocumentTagsModal')
3238

33-
/** Field type display labels */
34-
const FIELD_TYPE_LABELS: Record<string, string> = {
35-
text: 'Text',
36-
number: 'Number',
37-
date: 'Date',
38-
boolean: 'Boolean',
39-
}
40-
4139
/**
4240
* Gets the appropriate value when changing field types.
4341
* Clears value when type changes to allow placeholder to show.
@@ -462,6 +460,7 @@ export function DocumentTagsModal({
462460
setEditTagForm({ ...editTagForm, displayName: e.target.value })
463461
}
464462
placeholder='Enter tag name'
463+
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
465464
error={tagNameConflict}
466465
onKeyDown={(e) => {
467466
if (e.key === 'Enter' && canSaveTag) {
@@ -615,6 +614,7 @@ export function DocumentTagsModal({
615614
setEditTagForm({ ...editTagForm, displayName: e.target.value })
616615
}
617616
placeholder='Enter tag name'
617+
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
618618
error={tagNameConflict}
619619
onKeyDown={(e) => {
620620
if (e.key === 'Enter' && canSaveTag) {

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/base-tags-modal/base-tags-modal.tsx

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@ import {
1717
} from '@sim/emcn'
1818
import { createLogger } from '@sim/logger'
1919
import type { TagUsageData } from '@/lib/api/contracts/knowledge'
20-
import { SUPPORTED_FIELD_TYPES, TAG_SLOT_CONFIG } from '@/lib/knowledge/constants'
20+
import {
21+
FIELD_TYPE_LABELS,
22+
KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH,
23+
SUPPORTED_FIELD_TYPES,
24+
TAG_SLOT_CONFIG,
25+
} from '@/lib/knowledge/constants'
2126
import { getDocumentIcon } from '@/app/workspace/[workspaceId]/knowledge/components'
2227
import {
2328
type TagDefinition,
@@ -31,13 +36,6 @@ import {
3136

3237
const logger = createLogger('BaseTagsModal')
3338

34-
const FIELD_TYPE_LABELS: Record<string, string> = {
35-
text: 'Text',
36-
number: 'Number',
37-
date: 'Date',
38-
boolean: 'Boolean',
39-
}
40-
4139
interface DocumentListProps {
4240
documents: Array<{ id: string; name: string; tagValue: string }>
4341
totalCount: number
@@ -154,7 +152,9 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM
154152
isCreatingTag && !createTagMutation.isPending && hasTagNameConflict(createTagForm.displayName)
155153

156154
const canSaveTag = () => {
157-
return createTagForm.displayName.trim() && !hasTagNameConflict(createTagForm.displayName)
155+
return (
156+
createTagForm.displayName.trim().length > 0 && !hasTagNameConflict(createTagForm.displayName)
157+
)
158158
}
159159

160160
const getSlotUsageByFieldType = (fieldType: string): { used: number; max: number } => {
@@ -331,6 +331,7 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM
331331
setCreateTagForm({ ...createTagForm, displayName: e.target.value })
332332
}
333333
placeholder='Enter tag name'
334+
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
334335
error={Boolean(tagNameConflict)}
335336
onKeyDown={(e) => {
336337
if (e.key === 'Enter' && canSaveTag()) {

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,27 @@ import {
66
successResponseSchema,
77
} from '@/lib/api/contracts/knowledge/shared'
88
import { defineRouteContract } from '@/lib/api/contracts/types'
9+
import { KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH } from '@/lib/knowledge/constants'
910

1011
export const nextAvailableSlotQuerySchema = z.object({
1112
fieldType: z.string().min(1),
1213
})
1314

1415
export const createTagDefinitionBodySchema = z.object({
1516
tagSlot: z.string().min(1, 'Tag slot is required'),
16-
displayName: z.string().min(1, 'Display name is required'),
17+
displayName: z
18+
.string()
19+
.min(1, 'Display name is required')
20+
.max(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH, 'Display name too long'),
1721
fieldType: z.string().min(1, 'Invalid field type'),
1822
})
1923

2024
export const documentTagDefinitionInputSchema = z.object({
2125
tagSlot: z.string().min(1, 'Tag slot is required'),
22-
displayName: z.string().min(1, 'Display name is required').max(100, 'Display name too long'),
26+
displayName: z
27+
.string()
28+
.min(1, 'Display name is required')
29+
.max(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH, 'Display name too long'),
2330
fieldType: z.string().default('text'),
2431
_originalDisplayName: z.string().optional(),
2532
})

apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
type ServerToolContext,
2323
} from '@/lib/copilot/tools/server/base-tool'
2424
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
25+
import { KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH } from '@/lib/knowledge/constants'
2526
import {
2627
createSingleDocument,
2728
deleteDocument,
@@ -705,6 +706,12 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
705706
message: 'tagDisplayName is required for create_tag operation',
706707
}
707708
}
709+
if (args.tagDisplayName.length > KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH) {
710+
return {
711+
success: false,
712+
message: `tagDisplayName must be ${KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH} characters or less`,
713+
}
714+
}
708715

709716
const writeAccess = await checkKnowledgeBaseWriteAccess(
710717
args.knowledgeBaseId,
@@ -777,6 +784,15 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
777784
message: 'At least one of tagDisplayName or tagFieldType is required for update_tag',
778785
}
779786
}
787+
if (
788+
updateData.displayName &&
789+
updateData.displayName.length > KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH
790+
) {
791+
return {
792+
success: false,
793+
message: `tagDisplayName must be ${KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH} characters or less`,
794+
}
795+
}
780796

781797
const existingTag = await getTagDefinitionById(args.tagDefinitionId)
782798
if (!existingTag) {

apps/sim/lib/knowledge/constants.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ export type TagSlot = (typeof TAG_SLOTS)[number]
4343
/** Type for all tag slots */
4444
export type AllTagSlot = (typeof ALL_TAG_SLOTS)[number]
4545

46+
/**
47+
* Max character length for a tag display name, enforced on every write path (UI,
48+
* create API, bulk document API, copilot tools).
49+
*/
50+
export const KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH = 100
51+
4652
/** Type for number tag slots */
4753
export type NumberTagSlot = (typeof TAG_SLOT_CONFIG.number.slots)[number]
4854

0 commit comments

Comments
 (0)