Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { saveDocumentTagDefinitionsContract } from '@/lib/api/contracts/knowledg
import { parseRequest } from '@/lib/api/server'
import { getSession } from '@/lib/auth'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
import { getFieldTypeForSlot, SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
import {
cleanupUnusedTagDefinitions,
createOrUpdateTagDefinitionsBulk,
Expand Down Expand Up @@ -114,6 +114,16 @@ export const POST = withRouteHandler(
{ status: 400 }
)
}
/**
* Slot validity only, not slot/field-type agreement: this route also renames
* existing definitions, which resend whatever pair is already stored.
*/
if (getFieldTypeForSlot(def.tagSlot) === null) {
return NextResponse.json(
{ error: 'Invalid request data', details: `Unsupported tag slot: ${def.tagSlot}` },
{ status: 400 }
)
}
}

const bulkData: BulkTagDefinitionsData = {
Expand Down
60 changes: 60 additions & 0 deletions apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ vi.mock('@/lib/knowledge/tags/service', () => ({

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

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

const KB_ID = 'kb-victim'
Expand Down Expand Up @@ -140,5 +141,64 @@ describe('Knowledge Base Tag Definitions API Route', () => {
expect(mockCheckKnowledgeBaseWriteAccess).not.toHaveBeenCalled()
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
})

it('rejects a tag slot this schema has no column for', async () => {
authenticateAs('user-1', 'session')
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)

const response = await POST(
createMockRequest('POST', { ...CREATE_BODY, tagSlot: 'tag99' }),
params()
)

expect(response.status).toBe(400)
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
})

it('rejects a slot that belongs to a different field type', async () => {
authenticateAs('user-1', 'session')
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)

const response = await POST(
createMockRequest('POST', {
tagSlot: 'number1',
displayName: 'Mismatch',
fieldType: 'text',
}),
params()
)

expect(response.status).toBe(400)
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
})

it('rejects an unsupported field type', async () => {
authenticateAs('user-1', 'session')
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)

const response = await POST(
createMockRequest('POST', { ...CREATE_BODY, fieldType: 'nonsense' }),
params()
)

expect(response.status).toBe(400)
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
})

it('rejects a display name longer than the shared limit', async () => {
authenticateAs('user-1', 'session')
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted)

const response = await POST(
createMockRequest('POST', {
...CREATE_BODY,
displayName: 'a'.repeat(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH + 1),
}),
params()
)

expect(response.status).toBe(400)
expect(mockCreateTagDefinition).not.toHaveBeenCalled()
})
})
})
15 changes: 12 additions & 3 deletions apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { createTagDefinitionContract } from '@/lib/api/contracts/knowledge'
import { parseRequest } from '@/lib/api/server'
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
import { isValidSlotForFieldType } from '@/lib/knowledge/constants'
import { createTagDefinition, getTagDefinitions } from '@/lib/knowledge/tags/service'
import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils'

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

const validatedData = parsed.data.body
if (!(SUPPORTED_FIELD_TYPES as readonly string[]).includes(validatedData.fieldType)) {
/**
* The contract types `tagSlot` and `fieldType` as plain strings because
* tightening them to enums cascades into UI form state types, so the pair is
* checked here. Nothing downstream enforces it: the slot column is `text`
* (its Drizzle `enum` is types-only) and the service casts before inserting.
*/
if (!isValidSlotForFieldType(validatedData.tagSlot, validatedData.fieldType)) {
return NextResponse.json(
{ error: 'Invalid request data', details: 'Invalid field type' },
{
error: 'Invalid request data',
details: `Tag slot "${validatedData.tagSlot}" is not valid for field type "${validatedData.fieldType}"`,
},
{ status: 400 }
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import {
} from '@sim/emcn'
import { createLogger } from '@sim/logger'
import { formatDate } from '@sim/utils/formatting'
import { ALL_TAG_SLOTS, type AllTagSlot, MAX_TAG_SLOTS } from '@/lib/knowledge/constants'
import {
ALL_TAG_SLOTS,
type AllTagSlot,
FIELD_TYPE_LABELS,
KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH,
MAX_TAG_SLOTS,
} from '@/lib/knowledge/constants'
import type { DocumentTag } from '@/lib/knowledge/tags/types'
import type { DocumentData } from '@/lib/knowledge/types'
import {
Expand All @@ -30,14 +36,6 @@ import { useNextAvailableSlotMutation, useUpdateDocumentTags } from '@/hooks/que

const logger = createLogger('DocumentTagsModal')

/** Field type display labels */
const FIELD_TYPE_LABELS: Record<string, string> = {
text: 'Text',
number: 'Number',
date: 'Date',
boolean: 'Boolean',
}

/**
* Gets the appropriate value when changing field types.
* Clears value when type changes to allow placeholder to show.
Expand Down Expand Up @@ -462,6 +460,7 @@ export function DocumentTagsModal({
setEditTagForm({ ...editTagForm, displayName: e.target.value })
}
placeholder='Enter tag name'
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
error={tagNameConflict}
onKeyDown={(e) => {
if (e.key === 'Enter' && canSaveTag) {
Expand Down Expand Up @@ -615,6 +614,7 @@ export function DocumentTagsModal({
setEditTagForm({ ...editTagForm, displayName: e.target.value })
}
placeholder='Enter tag name'
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
error={tagNameConflict}
onKeyDown={(e) => {
if (e.key === 'Enter' && canSaveTag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import {
} from '@sim/emcn'
import { createLogger } from '@sim/logger'
import type { TagUsageData } from '@/lib/api/contracts/knowledge'
import { SUPPORTED_FIELD_TYPES, TAG_SLOT_CONFIG } from '@/lib/knowledge/constants'
import {
FIELD_TYPE_LABELS,
KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH,
SUPPORTED_FIELD_TYPES,
TAG_SLOT_CONFIG,
} from '@/lib/knowledge/constants'
import { getDocumentIcon } from '@/app/workspace/[workspaceId]/knowledge/components'
import {
type TagDefinition,
Expand All @@ -31,13 +36,6 @@ import {

const logger = createLogger('BaseTagsModal')

const FIELD_TYPE_LABELS: Record<string, string> = {
text: 'Text',
number: 'Number',
date: 'Date',
boolean: 'Boolean',
}

interface DocumentListProps {
documents: Array<{ id: string; name: string; tagValue: string }>
totalCount: number
Expand Down Expand Up @@ -154,7 +152,9 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM
isCreatingTag && !createTagMutation.isPending && hasTagNameConflict(createTagForm.displayName)

const canSaveTag = () => {
return createTagForm.displayName.trim() && !hasTagNameConflict(createTagForm.displayName)
return (
createTagForm.displayName.trim().length > 0 && !hasTagNameConflict(createTagForm.displayName)
)
}

const getSlotUsageByFieldType = (fieldType: string): { used: number; max: number } => {
Expand Down Expand Up @@ -331,6 +331,7 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM
setCreateTagForm({ ...createTagForm, displayName: e.target.value })
}
placeholder='Enter tag name'
maxLength={KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH}
error={Boolean(tagNameConflict)}
onKeyDown={(e) => {
if (e.key === 'Enter' && canSaveTag()) {
Expand Down
11 changes: 9 additions & 2 deletions apps/sim/lib/api/contracts/knowledge/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,27 @@ import {
successResponseSchema,
} from '@/lib/api/contracts/knowledge/shared'
import { defineRouteContract } from '@/lib/api/contracts/types'
import { KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH } from '@/lib/knowledge/constants'

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

export const createTagDefinitionBodySchema = z.object({
tagSlot: z.string().min(1, 'Tag slot is required'),
displayName: z.string().min(1, 'Display name is required'),
displayName: z
.string()
.min(1, 'Display name is required')
.max(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH, 'Display name too long'),
fieldType: z.string().min(1, 'Invalid field type'),
})

export const documentTagDefinitionInputSchema = z.object({
tagSlot: z.string().min(1, 'Tag slot is required'),
displayName: z.string().min(1, 'Display name is required').max(100, 'Display name too long'),
displayName: z
.string()
.min(1, 'Display name is required')
.max(KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH, 'Display name too long'),
fieldType: z.string().default('text'),
_originalDisplayName: z.string().optional(),
})
Expand Down
16 changes: 16 additions & 0 deletions apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
type ServerToolContext,
} from '@/lib/copilot/tools/server/base-tool'
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
import { KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH } from '@/lib/knowledge/constants'
import {
createSingleDocument,
deleteDocument,
Expand Down Expand Up @@ -705,6 +706,12 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
message: 'tagDisplayName is required for create_tag operation',
}
}
if (args.tagDisplayName.length > KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH) {
return {
success: false,
message: `tagDisplayName must be ${KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH} characters or less`,
}
}

const writeAccess = await checkKnowledgeBaseWriteAccess(
args.knowledgeBaseId,
Expand Down Expand Up @@ -777,6 +784,15 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
message: 'At least one of tagDisplayName or tagFieldType is required for update_tag',
}
}
if (
updateData.displayName &&
updateData.displayName.length > KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH
) {
return {
success: false,
message: `tagDisplayName must be ${KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH} characters or less`,
}
}

const existingTag = await getTagDefinitionById(args.tagDefinitionId)
if (!existingTag) {
Expand Down
6 changes: 6 additions & 0 deletions apps/sim/lib/knowledge/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ export type TagSlot = (typeof TAG_SLOTS)[number]
/** Type for all tag slots */
export type AllTagSlot = (typeof ALL_TAG_SLOTS)[number]

/**
* Max character length for a tag display name, enforced on every write path (UI,
* create API, bulk document API, copilot tools).
*/
export const KNOWLEDGE_TAG_DISPLAY_NAME_MAX_LENGTH = 100

/** Type for number tag slots */
export type NumberTagSlot = (typeof TAG_SLOT_CONFIG.number.slots)[number]

Expand Down
Loading