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
39 changes: 39 additions & 0 deletions apps/sim/lib/api/contracts/knowledge/documents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ import { describe, expect, it } from 'vitest'
import { z } from 'zod'
import {
bulkCreateDocumentsBodySchema,
createDocumentBodySchema,
documentDataSchema,
listKnowledgeDocumentsQuerySchema,
parseDocumentTagFiltersParam,
updateDocumentBodySchema,
upsertDocumentBodySchema,
} from '@/lib/api/contracts/knowledge/documents'
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
import { getDocumentIndexingStatus } from '@/lib/knowledge/documents/types'

describe('document processing response compatibility', () => {
Expand Down Expand Up @@ -255,3 +258,39 @@ describe('internal document processingOptions', () => {
})
})
})

describe('document filename and tag bounds', () => {
const base = { fileUrl: 'https://example.com/a.txt', fileSize: 1, mimeType: 'text/plain' }
const atLimit = 'a'.repeat(MAX_DOCUMENT_INDEXED_TEXT_LENGTH)
const overLimit = `${atLimit}a`

it('accepts a filename and tag exactly at the indexed-text limit', () => {
expect(
createDocumentBodySchema.safeParse({ ...base, filename: atLimit, tag1: atLimit }).success
).toBe(true)
})

it('rejects a filename over the limit on create, upsert, and update with a descriptive message', () => {
for (const schema of [
createDocumentBodySchema,
upsertDocumentBodySchema,
updateDocumentBodySchema,
]) {
const result = schema.safeParse({ ...base, filename: overLimit })
expect(result.success).toBe(false)
expect(result.error?.issues[0]?.message).toBe(
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
)
}
})

it('rejects a tag value over the limit on create and update', () => {
for (const schema of [createDocumentBodySchema, updateDocumentBodySchema]) {
const result = schema.safeParse({ ...base, filename: 'a.txt', tag3: overLimit })
expect(result.success).toBe(false)
expect(result.error?.issues[0]?.message).toBe(
`Tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
)
}
})
})
67 changes: 49 additions & 18 deletions apps/sim/lib/api/contracts/knowledge/documents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import {
import { privateSecretProvenanceBundleSchema } from '@/lib/api/contracts/primitives'
import { defineRouteContract } from '@/lib/api/contracts/types'
import { PRIVATE_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata'
import { getFieldTypeForSlot, MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE } from '@/lib/knowledge/constants'
import {
getFieldTypeForSlot,
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE,
} from '@/lib/knowledge/constants'
import { DOCUMENT_PROCESSING_STATUSES } from '@/lib/knowledge/documents/types'
import { getOperatorsForFieldType, isValidFilterValue } from '@/lib/knowledge/filters/types'
import { knowledgeDocumentUploadMetadataSchema } from '@/lib/knowledge/upload-metadata'
Expand Down Expand Up @@ -115,18 +119,32 @@ export function parseDocumentTagFiltersParam(
return z.array(documentTagFilterSchema).parse(JSON.parse(value))
}

/** A text tag value that fits its index row; see {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH}. */
const documentTagValueSchema = z
.string()
.max(
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
`Tag values cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
)

export const createDocumentBodySchema = z.object({
filename: z.string().min(1, 'Filename is required'),
filename: z
Comment thread
waleedlatif1 marked this conversation as resolved.
.string()
.min(1, 'Filename is required')
.max(
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
),
fileUrl: knowledgeDocumentFileUrlSchema,
fileSize: z.number().min(1, 'File size must be greater than 0'),
mimeType: z.string().min(1, 'MIME type is required'),
tag1: z.string().optional(),
tag2: z.string().optional(),
tag3: z.string().optional(),
tag4: z.string().optional(),
tag5: z.string().optional(),
tag6: z.string().optional(),
tag7: z.string().optional(),
tag1: documentTagValueSchema.optional(),
tag2: documentTagValueSchema.optional(),
tag3: documentTagValueSchema.optional(),
tag4: documentTagValueSchema.optional(),
tag5: documentTagValueSchema.optional(),
tag6: documentTagValueSchema.optional(),
tag7: documentTagValueSchema.optional(),
documentTagsData: z.string().optional(),
})

Expand Down Expand Up @@ -165,7 +183,13 @@ export type SingleCreateDocumentBody = z.input<typeof singleCreateDocumentBodySc

export const upsertDocumentBodySchema = z.object({
documentId: z.string().optional(),
filename: z.string().min(1, 'Filename is required'),
filename: z
.string()
.min(1, 'Filename is required')
.max(
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
),
fileUrl: knowledgeDocumentFileUrlSchema,
fileSize: z.number().min(1, 'File size must be greater than 0'),
mimeType: z.string().min(1, 'MIME type is required'),
Expand Down Expand Up @@ -196,7 +220,14 @@ export const bulkCreateDocumentsResponseSchema = z.object({
})

export const updateDocumentBodySchema = z.object({
filename: z.string().min(1, 'Filename is required').optional(),
filename: z
.string()
.min(1, 'Filename is required')
.max(
MAX_DOCUMENT_INDEXED_TEXT_LENGTH,
`Filename cannot exceed ${MAX_DOCUMENT_INDEXED_TEXT_LENGTH} characters`
)
.optional(),
enabled: z.boolean().optional(),
chunkCount: z.number().min(0).optional(),
tokenCount: z.number().min(0).optional(),
Expand All @@ -205,13 +236,13 @@ export const updateDocumentBodySchema = z.object({
processingError: z.string().optional(),
markFailedDueToTimeout: z.boolean().optional(),
retryProcessing: z.boolean().optional(),
tag1: z.string().optional(),
tag2: z.string().optional(),
tag3: z.string().optional(),
tag4: z.string().optional(),
tag5: z.string().optional(),
tag6: z.string().optional(),
tag7: z.string().optional(),
tag1: documentTagValueSchema.optional(),
tag2: documentTagValueSchema.optional(),
tag3: documentTagValueSchema.optional(),
tag4: documentTagValueSchema.optional(),
tag5: documentTagValueSchema.optional(),
tag6: documentTagValueSchema.optional(),
tag7: documentTagValueSchema.optional(),
number1: z.string().optional(),
number2: z.string().optional(),
number3: z.string().optional(),
Expand Down
6 changes: 4 additions & 2 deletions apps/sim/lib/knowledge/application/documents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import {
toKnowledgeTagFilterConditions,
} from '@/lib/knowledge/tags/filter-resolution'
import { getDocumentTagDefinitions } from '@/lib/knowledge/tags/service'
import { validateTagValue } from '@/lib/knowledge/tags/utils'
import { validateTagValue, validateTagValueLength } from '@/lib/knowledge/tags/utils'
import { StorageService } from '@/lib/uploads'
import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager'
import { recordKnowledgeBaseFileOwnership } from '@/lib/uploads/server/metadata'
Expand Down Expand Up @@ -252,7 +252,9 @@ async function resolveKnowledgeDocumentTagValueUpdates(
`Tag "${definition.displayName}" requires a value; use null to clear it`
)
}
const validationError = validateTagValue(definition.displayName, value, definition.fieldType)
const validationError =
validateTagValueLength(definition.displayName, value) ??
validateTagValue(definition.displayName, value, definition.fieldType)
if (validationError) {
throw new OrchestrationError('validation', validationError)
}
Expand Down
56 changes: 55 additions & 1 deletion apps/sim/lib/knowledge/connectors/sync-persistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,24 @@ vi.mock('@/lib/knowledge/documents/storage-cleanup', () => ({
}),
isKnowledgeBaseOwnedStorageKey: (key: string) => key.startsWith('kb/'),
}))
vi.mock('@/connectors/registry.server', () => ({ CONNECTOR_REGISTRY: {} }))
vi.mock('@/connectors/registry.server', () => ({
CONNECTOR_REGISTRY: {
fixture: {
mapTags: (metadata: Record<string, unknown>) => ({
label: metadata.label,
owner: metadata.owner,
}),
},
},
}))

import { MAX_ACL_TOKENS } from '@/lib/knowledge/access/tokens'
import { getConnectorFailureDiagnostic } from '@/lib/knowledge/connectors/connector-error'
import {
addDocument,
persistDocumentAcls,
persistSourceDocumentFailures,
resolveTagMapping,
} from '@/lib/knowledge/connectors/sync-persistence'

const CONNECTOR = 'connector-1'
Expand Down Expand Up @@ -350,6 +360,18 @@ describe('persistSourceDocumentFailures', () => {
expect(JSON.stringify(dbChainMockFns.set.mock.calls)).not.toContain('private body')
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
})
it('bounds a source title that would exceed the filename index row limit', async () => {
leaseHeld()
const title = 'x'.repeat(5000)
await persistSourceDocumentFailures({
...input,
documents: [{ ...input.documents[0], title }],
priorByExternalId: new Map(),
})
Comment thread
waleedlatif1 marked this conversation as resolved.
const [rows] = dbChainMockFns.values.mock.calls[0] as [Array<{ filename: string }>]
expect(rows[0].filename).toBe(`${'x'.repeat(509)}...`)
expect(rows[0].filename.length).toBe(512)
})
it('refuses to commit a failure under a reclaimed lease', async () => {
queueTableRows(schemaMock.knowledgeBase, [{ id: 'kb' }])
await expect(
Expand Down Expand Up @@ -416,3 +438,35 @@ describe('organization source cache persistence', () => {
expect(mockUploadFile).not.toHaveBeenCalled()
})
})

describe('resolveTagMapping', () => {
it('bounds a mapped tag value that would exceed its index row limit and keeps a short one intact', () => {
const tags = resolveTagMapping(
'fixture',
{ label: 'y'.repeat(5000), owner: 'Purchasing' },
{ tagSlotMapping: { label: 'tag1', owner: 'tag2' } }
)
expect(tags?.tag1).toBe(`${'y'.repeat(509)}...`)
expect(tags?.tag2).toBe('Purchasing')
})

it('keeps a value exactly at the limit untouched', () => {
const atLimit = 'z'.repeat(512)
const tags = resolveTagMapping(
'fixture',
{ label: atLimit },
{ tagSlotMapping: { label: 'tag1' } }
)
expect(tags?.tag1).toBe(atLimit)
})

it('cuts by code point so a bounded value never ends in half a surrogate pair', () => {
const tags = resolveTagMapping(
'fixture',
{ label: '\u{1F600}'.repeat(600) },
{ tagSlotMapping: { label: 'tag1' } }
)
expect(tags?.tag1).toBe(`${'\u{1F600}'.repeat(254)}...`)
expect(tags?.tag1?.length).toBeLessThanOrEqual(512)
})
})
28 changes: 24 additions & 4 deletions apps/sim/lib/knowledge/connectors/sync-persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { document, embedding, knowledgeBase, knowledgeConnector } from '@sim/db/
import { createLogger } from '@sim/logger'
import { chunkArray } from '@sim/utils/helpers'
import { generateId } from '@sim/utils/id'
import { truncateAtCodePoint } from '@sim/utils/string'
import { and, eq, exists, inArray, isNull, lt, or, sql } from 'drizzle-orm'
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
import type { DbOrTx } from '@/lib/db/types'
Expand All @@ -18,6 +19,7 @@ import type { ConnectorFailureDiagnostic } from '@/lib/knowledge/connectors/conn
import { resolveSourceModifiedAt } from '@/lib/knowledge/connectors/source-modified-at'
import { SOURCE_CONTENT_ERROR } from '@/lib/knowledge/connectors/sync-limits'
import { assertSyncLeaseHeldInTx, type SyncWriteLease } from '@/lib/knowledge/connectors/sync-lock'
import { MAX_DOCUMENT_INDEXED_TEXT_LENGTH } from '@/lib/knowledge/constants'
import type { DocumentData } from '@/lib/knowledge/documents/service'
import { enqueueKnowledgeStorageCleanup } from '@/lib/knowledge/documents/storage-cleanup'
import {
Expand Down Expand Up @@ -173,6 +175,23 @@ export async function persistDocumentAcls(

const MAX_SAFE_TITLE_LENGTH = 200

/** The suffix a cut value carries, counted inside {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH}. */
const INDEXED_TEXT_CUT_SUFFIX = '...'

/**
* Source titles and mapped tag values are untrusted machine input with no caller to refuse them,
* so they are cut to {@link MAX_DOCUMENT_INDEXED_TEXT_LENGTH} code units, suffix included, and
* never inside a surrogate pair. The result always passes the document APIs' own bound.
*/
function boundIndexedText(value: string): string {
if (value.length <= MAX_DOCUMENT_INDEXED_TEXT_LENGTH) return value
return truncateAtCodePoint(
value,
MAX_DOCUMENT_INDEXED_TEXT_LENGTH - INDEXED_TEXT_CUT_SUFFIX.length,
INDEXED_TEXT_CUT_SUFFIX
)
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}

function sanitizeStorageTitle(title: string): string {
return title.replace(/[^a-zA-Z0-9.-]/g, '_').slice(0, MAX_SAFE_TITLE_LENGTH)
}
Expand Down Expand Up @@ -250,7 +269,8 @@ export function resolveTagMapping(
const result: Partial<DocumentTags> = {}
for (const [semanticKey, slot] of Object.entries(mapping)) {
const value = semanticTags[semanticKey]
;(result as Record<string, unknown>)[slot] = value != null ? value : null
;(result as Record<string, unknown>)[slot] =
typeof value === 'string' ? boundIndexedText(value) : (value ?? null)
}
return result
}
Expand Down Expand Up @@ -278,7 +298,7 @@ function buildSkippedDocumentRow(
return {
id: generateId(),
knowledgeBaseId,
filename: extDoc.title,
filename: boundIndexedText(extDoc.title),
fileUrl: '',
storageKey: null,
/** No artifact was stored; a provider's reported source size is not local storage usage. */
Expand Down Expand Up @@ -630,7 +650,7 @@ export async function addDocument(
await tx.insert(document).values({
id: documentId,
knowledgeBaseId,
filename: extDoc.title,
filename: boundIndexedText(extDoc.title),
fileUrl,
storageKey: fileInfo.key,
fileSize: artifact.bytes.length,
Expand Down Expand Up @@ -745,7 +765,7 @@ export async function updateDocument(
await tx
.update(document)
.set({
filename: extDoc.title,
filename: boundIndexedText(extDoc.title),
fileUrl,
storageKey: fileInfo.key,
fileSize: artifact.bytes.length,
Expand Down
8 changes: 8 additions & 0 deletions apps/sim/lib/knowledge/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
/** Max character length for a knowledge base description, enforced at every layer (UI, internal API, v1 API). */
export const KNOWLEDGE_BASE_DESCRIPTION_MAX_LENGTH = 10_000

/**
* Max character length for a document's filename and text tag values. Both sit under btree
* indexes, and Postgres refuses an index row past about 2.7 KB (SQLSTATE 54000); 512 characters
* keeps a four-byte-per-character value inside that ceiling. Connectors truncate source titles to
* it; the document APIs reject longer input.
*/
export const MAX_DOCUMENT_INDEXED_TEXT_LENGTH = 512

/** Hard bound for path-indexed knowledge folder trees and recursive cascades. */
export const MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE = MAX_FOLDERS_PER_WORKSPACE

Expand Down
5 changes: 4 additions & 1 deletion apps/sim/lib/knowledge/documents/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ import {
parseNumberValue,
uncompilableTagFilterError,
validateTagValue,
validateTagValueLength,
} from '@/lib/knowledge/tags/utils'
import type { ProcessedDocumentTags } from '@/lib/knowledge/types'
import { embeddingVectorValues } from '@/lib/knowledge/vector-columns'
Expand Down Expand Up @@ -552,7 +553,9 @@ function resolveDocumentTags(

const rawValue = typeof tag.value === 'string' ? tag.value.trim() : tag.value
const actualFieldType = existingDef.fieldType || fieldType
const validationError = validateTagValue(tagName, String(rawValue), actualFieldType)
const validationError =
validateTagValueLength(tagName, String(rawValue)) ??
validateTagValue(tagName, String(rawValue), actualFieldType)
if (validationError) {
typeErrors.push(validationError)
}
Expand Down
15 changes: 14 additions & 1 deletion apps/sim/lib/knowledge/tags/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { coerceTagFilterValue, validateTagValue } from '@/lib/knowledge/tags/utils'
import {
coerceTagFilterValue,
validateTagValue,
validateTagValueLength,
} from '@/lib/knowledge/tags/utils'

describe('coerceTagFilterValue', () => {
it('accepts exactly what validateTagValue accepts', () => {
Expand Down Expand Up @@ -68,3 +72,12 @@ describe('validateTagValue', () => {
expect(validateTagValue('name', 'anything', 'json')).toBeNull()
})
})

describe('validateTagValueLength', () => {
it('accepts a value at the indexed-text limit and names the tag past it', () => {
expect(validateTagValueLength('Labels', 'a'.repeat(512))).toBeNull()
expect(validateTagValueLength('Labels', 'a'.repeat(513))).toBe(
'Tag "Labels" cannot exceed 512 characters'
)
})
})
Loading
Loading