Skip to content

Commit 80e7843

Browse files
committed
fix(security): address PR review — document IDOR, log count, token split
- knowledge-base delete_document/update_document: verify each document belongs to the claimed knowledgeBaseId via checkDocumentWriteAccess (was: trusted args.knowledgeBaseId without binding it to the document) - multipart batch complete: log verifiedEntries.length instead of raw client-supplied data.uploads.length - upload-token: reject tokens with !=2 dot-delimited segments
1 parent 873c02f commit 80e7843

3 files changed

Lines changed: 24 additions & 19 deletions

File tree

apps/sim/app/api/files/multipart/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
239239
})
240240
)
241241

242-
logger.info(`Completed ${data.uploads.length} multipart uploads`)
242+
logger.info(`Completed ${verifiedEntries.length} multipart uploads`)
243243
return NextResponse.json({ results })
244244
}
245245

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

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ import {
3737
import { StorageService } from '@/lib/uploads'
3838
import { resolveWorkspaceFileReference } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
3939
import { getQueryStrategy, handleVectorOnlySearch } from '@/app/api/knowledge/search/utils'
40-
import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils'
40+
import {
41+
checkDocumentWriteAccess,
42+
checkKnowledgeBaseAccess,
43+
checkKnowledgeBaseWriteAccess,
44+
} from '@/app/api/knowledge/utils'
4145

4246
const logger = createLogger('KnowledgeBaseServerTool')
4347

@@ -485,23 +489,21 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
485489
}
486490
}
487491

488-
const writeAccess = await checkKnowledgeBaseWriteAccess(
489-
args.knowledgeBaseId,
490-
context.userId
491-
)
492-
if (!writeAccess.hasAccess) {
493-
return {
494-
success: false,
495-
message: `Knowledge base with ID "${args.knowledgeBaseId}" not found`,
496-
}
497-
}
498-
499492
const deleted: string[] = []
500493
const failed: string[] = []
501494

502495
for (const docId of docIds) {
503-
const requestId = generateId().slice(0, 8)
504496
assertNotAborted()
497+
const docAccess = await checkDocumentWriteAccess(
498+
args.knowledgeBaseId,
499+
docId,
500+
context.userId
501+
)
502+
if (!docAccess.hasAccess) {
503+
failed.push(docId)
504+
continue
505+
}
506+
const requestId = generateId().slice(0, 8)
505507
const result = await deleteDocument(docId, requestId)
506508
if (result.success) {
507509
deleted.push(docId)
@@ -537,14 +539,15 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg
537539
message: 'At least one of filename or enabled is required for update_document',
538540
}
539541
}
540-
const writeAccess = await checkKnowledgeBaseWriteAccess(
542+
const docAccess = await checkDocumentWriteAccess(
541543
args.knowledgeBaseId,
544+
args.documentId,
542545
context.userId
543546
)
544-
if (!writeAccess.hasAccess) {
547+
if (!docAccess.hasAccess) {
545548
return {
546549
success: false,
547-
message: `Knowledge base with ID "${args.knowledgeBaseId}" not found`,
550+
message: `Document with ID "${args.documentId}" not found`,
548551
}
549552
}
550553
const requestId = generateId().slice(0, 8)

apps/sim/lib/uploads/core/upload-token.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,12 @@ export type UploadTokenVerification =
4141
| { valid: false }
4242

4343
export function verifyUploadToken(token: string): UploadTokenVerification {
44-
if (typeof token !== 'string' || !token.includes('.')) {
44+
if (typeof token !== 'string') {
4545
return { valid: false }
4646
}
47-
const [encoded, signature] = token.split('.', 2)
47+
const parts = token.split('.')
48+
if (parts.length !== 2) return { valid: false }
49+
const [encoded, signature] = parts
4850
if (!encoded || !signature) return { valid: false }
4951

5052
const expected = sign(encoded)

0 commit comments

Comments
 (0)