diff --git a/apps/sim/app/api/files/authorization.ts b/apps/sim/app/api/files/authorization.ts index 73c3b1e262a..b2cc73c429f 100644 --- a/apps/sim/app/api/files/authorization.ts +++ b/apps/sim/app/api/files/authorization.ts @@ -28,6 +28,21 @@ interface AuthorizationResult { workspaceId?: string } +type WorkspacePermission = 'read' | 'write' | 'admin' + +/** + * Whether a resolved workspace permission satisfies a file operation. Read and + * download paths accept any membership; destructive operations (`requireWrite`) + * require write or admin, matching the permission needed to create the file. + */ +function workspacePermissionSatisfies( + permission: WorkspacePermission | null, + requireWrite: boolean +): boolean { + if (permission === null) return false + return requireWrite ? permission === 'write' || permission === 'admin' : true +} + /** * Lookup workspace file by storage key from database * @param key Storage key to lookup @@ -117,11 +132,13 @@ export async function verifyFileAccess( userId: string, customConfig?: StorageConfig, context?: StorageContext | 'general', - isLocal?: boolean + isLocal?: boolean, + options?: { requireWrite?: boolean } ): Promise { + const requireWrite = options?.requireWrite ?? false try { if (context === 'general') { - return await verifyRegularFileAccess(cloudKey, userId, customConfig, isLocal) + return await verifyRegularFileAccess(cloudKey, userId, customConfig, isLocal, requireWrite) } // Infer context from key if not explicitly provided @@ -139,12 +156,12 @@ export async function verifyFileAccess( // 1. Workspace / mothership files: Check database first (most reliable for both local and cloud) if (inferredContext === 'workspace' || inferredContext === 'mothership') { - return await verifyWorkspaceFileAccess(cloudKey, userId, customConfig, isLocal) + return await verifyWorkspaceFileAccess(cloudKey, userId, customConfig, isLocal, requireWrite) } // 2. Execution files: workspace_id/workflow_id/execution_id/filename if (inferredContext === 'execution') { - return await verifyExecutionFileAccess(cloudKey, userId, customConfig) + return await verifyExecutionFileAccess(cloudKey, userId, customConfig, requireWrite) } // 3. Copilot files: Check database first, then metadata, then path pattern (legacy) @@ -159,12 +176,12 @@ export async function verifyFileAccess( // 5. Chat files: chat/filename if (inferredContext === 'chat') { - return await verifyChatFileAccess(cloudKey, userId, customConfig) + return await verifyChatFileAccess(cloudKey, userId, customConfig, requireWrite) } // 6. Regular uploads: UUID-filename or timestamp-filename // Check metadata for userId/workspaceId, or database for workspace files - return await verifyRegularFileAccess(cloudKey, userId, customConfig, isLocal) + return await verifyRegularFileAccess(cloudKey, userId, customConfig, isLocal, requireWrite) } catch (error) { logger.error('Error verifying file access:', { cloudKey, userId, error }) // Deny access on error to be safe @@ -180,7 +197,8 @@ async function verifyWorkspaceFileAccess( cloudKey: string, userId: string, customConfig?: StorageConfig, - isLocal?: boolean + isLocal?: boolean, + requireWrite = false ): Promise { try { const anyWorkspaceFileRecord = await getFileMetadataByKey(cloudKey, 'workspace', { @@ -202,7 +220,7 @@ async function verifyWorkspaceFileAccess( 'workspace', workspaceFileRecord.workspaceId ) - if (permission !== null) { + if (workspacePermissionSatisfies(permission, requireWrite)) { logger.debug('Workspace file access granted (database lookup)', { userId, workspaceId: workspaceFileRecord.workspaceId, @@ -225,7 +243,7 @@ async function verifyWorkspaceFileAccess( if (workspaceId) { const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - if (permission !== null) { + if (workspacePermissionSatisfies(permission, requireWrite)) { logger.debug('Workspace file access granted (metadata)', { userId, workspaceId, @@ -257,7 +275,8 @@ async function verifyWorkspaceFileAccess( async function verifyExecutionFileAccess( cloudKey: string, userId: string, - customConfig?: StorageConfig + customConfig?: StorageConfig, + requireWrite = false ): Promise { const parts = cloudKey.split('/') @@ -285,7 +304,7 @@ async function verifyExecutionFileAccess( } const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - if (permission === null) { + if (!workspacePermissionSatisfies(permission, requireWrite)) { logger.warn('User does not have workspace access for execution file', { userId, workspaceId, @@ -502,7 +521,8 @@ export async function verifyKBFileWriteAccess(cloudKey: string, userId: string): async function verifyChatFileAccess( cloudKey: string, userId: string, - customConfig?: StorageConfig + customConfig?: StorageConfig, + requireWrite = false ): Promise { try { const config: StorageConfig = customConfig || (await getChatStorageConfig()) @@ -516,7 +536,7 @@ async function verifyChatFileAccess( } const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - if (permission === null) { + if (!workspacePermissionSatisfies(permission, requireWrite)) { logger.warn('User does not have workspace access for chat file', { userId, workspaceId, @@ -542,7 +562,8 @@ async function verifyRegularFileAccess( cloudKey: string, userId: string, customConfig?: StorageConfig, - isLocal?: boolean + isLocal?: boolean, + requireWrite = false ): Promise { try { // Priority 1: Check if this might be a workspace file (check database) @@ -554,7 +575,7 @@ async function verifyRegularFileAccess( 'workspace', workspaceFileRecord.workspaceId ) - if (permission !== null) { + if (workspacePermissionSatisfies(permission, requireWrite)) { logger.debug('Regular file access granted (workspace file from database)', { userId, workspaceId: workspaceFileRecord.workspaceId, @@ -589,7 +610,7 @@ async function verifyRegularFileAccess( // If file has workspaceId, verify workspace membership if (workspaceId) { const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - if (permission !== null) { + if (workspacePermissionSatisfies(permission, requireWrite)) { logger.debug('Regular file access granted (workspace membership)', { userId, workspaceId, diff --git a/apps/sim/app/api/files/delete/route.ts b/apps/sim/app/api/files/delete/route.ts index 18b8b63ee95..c483faa3c6b 100644 --- a/apps/sim/app/api/files/delete/route.ts +++ b/apps/sim/app/api/files/delete/route.ts @@ -64,8 +64,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const storageContext: StorageContext = context || inferContextFromKey(key) - // KB deletes are binding-only and require write/admin on the owning workspace; - // they never use the transitional read fallback that file serving allows. + // Deletes require write/admin on the owning workspace (owner-scoped files + // like copilot/regular uploads still authorize by ownership). KB deletes + // are binding-only and never use the transitional read fallback that file + // serving allows. const hasAccess = storageContext === 'knowledge-base' ? await verifyKBFileWriteAccess(key, userId) @@ -74,7 +76,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { userId, undefined, // customConfig storageContext, // context - !hasCloudStorage() // isLocal + !hasCloudStorage(), // isLocal + { requireWrite: true } ) if (!hasAccess) { diff --git a/apps/sim/app/api/files/multipart/route.ts b/apps/sim/app/api/files/multipart/route.ts index 73fe863f0a6..fbdb4e4016a 100644 --- a/apps/sim/app/api/files/multipart/route.ts +++ b/apps/sim/app/api/files/multipart/route.ts @@ -24,7 +24,7 @@ import { type UploadTokenPayload, verifyUploadToken, } from '@/lib/uploads/core/upload-token' -import { insertFileMetadata } from '@/lib/uploads/server/metadata' +import { recordKnowledgeBaseFileOwnership } from '@/lib/uploads/server/metadata' import { QUOTA_EXEMPT_STORAGE_CONTEXTS, type StorageConfig } from '@/lib/uploads/shared/types' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' @@ -96,11 +96,10 @@ const recordKnowledgeBaseOwnership = async ( if (payload.context !== 'knowledge-base' || !payload.workspaceId) { return } - await insertFileMetadata({ + await recordKnowledgeBaseFileOwnership({ key, userId: payload.userId, workspaceId: payload.workspaceId, - context: 'knowledge-base', originalName: payload.fileName ?? key.split('/').pop() ?? key, contentType: payload.contentType ?? 'application/octet-stream', size: typeof payload.fileSize === 'number' ? payload.fileSize : 0, diff --git a/apps/sim/app/api/files/presigned/batch/route.ts b/apps/sim/app/api/files/presigned/batch/route.ts index 2d4791584f5..846c381d051 100644 --- a/apps/sim/app/api/files/presigned/batch/route.ts +++ b/apps/sim/app/api/files/presigned/batch/route.ts @@ -13,7 +13,7 @@ import { generateBatchPresignedUploadUrls, hasCloudStorage, } from '@/lib/uploads/core/storage-service' -import { insertFileMetadataMany } from '@/lib/uploads/server/metadata' +import { recordKnowledgeBaseFileOwnershipMany } from '@/lib/uploads/server/metadata' import { validateFileType } from '@/lib/uploads/utils/validation' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' import { createErrorResponse } from '@/app/api/files/utils' @@ -151,12 +151,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => { if (uploadType === 'knowledge-base' && knowledgeBaseWorkspaceId) { const ownerWorkspaceId = knowledgeBaseWorkspaceId - await insertFileMetadataMany( + await recordKnowledgeBaseFileOwnershipMany( presignedUrls.map((urlResponse, index) => ({ key: urlResponse.key, userId: sessionUserId, workspaceId: ownerWorkspaceId, - context: 'knowledge-base', originalName: files[index].fileName, contentType: files[index].contentType, size: files[index].fileSize, diff --git a/apps/sim/app/api/files/presigned/route.test.ts b/apps/sim/app/api/files/presigned/route.test.ts index 0a14699d9f9..b8c3d046573 100644 --- a/apps/sim/app/api/files/presigned/route.test.ts +++ b/apps/sim/app/api/files/presigned/route.test.ts @@ -93,6 +93,8 @@ vi.mock('@/lib/uploads/contexts/execution/utils', () => ({ vi.mock('@/lib/uploads/server/metadata', () => ({ insertFileMetadata: mockInsertFileMetadata, + recordKnowledgeBaseFileOwnership: (ownership: Record) => + mockInsertFileMetadata({ ...ownership, context: 'knowledge-base' }), })) vi.mock('@/lib/uploads/utils/file-utils', () => ({ diff --git a/apps/sim/app/api/files/presigned/route.ts b/apps/sim/app/api/files/presigned/route.ts index ec919236522..99c1eec2db6 100644 --- a/apps/sim/app/api/files/presigned/route.ts +++ b/apps/sim/app/api/files/presigned/route.ts @@ -11,7 +11,7 @@ import { USE_BLOB_STORAGE } from '@/lib/uploads/config' import { generateExecutionFileKey } from '@/lib/uploads/contexts/execution/utils' import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { generatePresignedUploadUrl, hasCloudStorage } from '@/lib/uploads/core/storage-service' -import { insertFileMetadata } from '@/lib/uploads/server/metadata' +import { insertFileMetadata, recordKnowledgeBaseFileOwnership } from '@/lib/uploads/server/metadata' import { isImageFileType } from '@/lib/uploads/utils/file-utils' import { validateAttachmentFileType, validateFileType } from '@/lib/uploads/utils/validation' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' @@ -278,11 +278,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => { metadata: { workspaceId }, }) - await insertFileMetadata({ + await recordKnowledgeBaseFileOwnership({ key: presignedUrlResponse.key, userId: sessionUserId, workspaceId, - context: 'knowledge-base', originalName: fileName, contentType, size: fileSize, diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index a60e552dc06..95cb3d53b05 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -465,22 +465,27 @@ describe('Individual Folder API Route', () => { expect(response.status).toBe(403) const data = await response.json() - expect(data).toHaveProperty('error', 'Admin access required to delete folders') + expect(data).toHaveProperty('error', 'Write or Admin access required to delete folders') }) - it('should return 403 when user has only write permissions for delete', async () => { + it('should allow folder deletion for write permissions', async () => { mockAuthenticatedUser() mockGetUserEntityPermissions.mockResolvedValue('write') + mockDbRef.current = createFolderDbMock({ + folderLookupResult: mockFolder, + }) + const req = createMockRequest('DELETE') const params = Promise.resolve({ id: 'folder-1' }) const response = await DELETE(req, { params }) - expect(response.status).toBe(403) + expect(response.status).toBe(200) const data = await response.json() - expect(data).toHaveProperty('error', 'Admin access required to delete folders') + expect(data).toHaveProperty('success', true) + expect(mockPerformDeleteFolder).toHaveBeenCalled() }) it('should allow folder deletion for admin permissions', async () => { diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index bc622793bc4..26d5f218005 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -140,9 +140,9 @@ export const DELETE = withRouteHandler( existingFolder.workspaceId ) - if (workspacePermission !== 'admin') { + if (!workspacePermission || workspacePermission === 'read') { return NextResponse.json( - { error: 'Admin access required to delete folders' }, + { error: 'Write or Admin access required to delete folders' }, { status: 403 } ) } diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts index 10594ff6635..ba225c3ecf4 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]/route.ts @@ -6,7 +6,7 @@ import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { deleteChunk, updateChunk } from '@/lib/knowledge/chunks/service' -import { checkChunkAccess } from '@/app/api/knowledge/utils' +import { checkChunkAccess, checkChunkWriteAccess } from '@/app/api/knowledge/utils' const logger = createLogger('ChunkByIdAPI') @@ -75,7 +75,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const accessCheck = await checkChunkAccess( + const accessCheck = await checkChunkWriteAccess( knowledgeBaseId, documentId, chunkId, @@ -147,7 +147,7 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const accessCheck = await checkChunkAccess( + const accessCheck = await checkChunkWriteAccess( knowledgeBaseId, documentId, chunkId, diff --git a/apps/sim/app/api/knowledge/utils.ts b/apps/sim/app/api/knowledge/utils.ts index 9dac8ffdb8e..316f8af3011 100644 --- a/apps/sim/app/api/knowledge/utils.ts +++ b/apps/sim/app/api/knowledge/utils.ts @@ -435,3 +435,72 @@ export async function checkChunkAccess( knowledgeBase: kbAccess.knowledgeBase!, } } + +/** + * Check if a user has write access to a chunk. + * + * Mirrors `checkChunkAccess` but requires write/admin on the knowledge base's + * workspace (or KB ownership for legacy KBs), matching the permission needed to + * create chunks. Used for chunk mutation (update and delete) so those operations + * require the same permission as creation rather than read. + */ +export async function checkChunkWriteAccess( + knowledgeBaseId: string, + documentId: string, + chunkId: string, + userId: string +): Promise { + const kbAccess = await checkKnowledgeBaseWriteAccess(knowledgeBaseId, userId) + + if (!kbAccess.hasAccess) { + return { + hasAccess: false, + notFound: kbAccess.notFound, + reason: kbAccess.notFound ? 'Knowledge base not found' : 'Unauthorized knowledge base access', + } + } + + const doc = await db + .select() + .from(document) + .where( + and( + eq(document.id, documentId), + eq(document.knowledgeBaseId, knowledgeBaseId), + eq(document.userExcluded, false), + isNull(document.archivedAt), + isNull(document.deletedAt) + ) + ) + .limit(1) + + if (doc.length === 0) { + return { hasAccess: false, notFound: true, reason: 'Document not found' } + } + + const docData = doc[0] as DocumentData + + if (docData.processingStatus !== 'completed') { + return { + hasAccess: false, + reason: `Document is not ready for access (status: ${docData.processingStatus})`, + } + } + + const chunk = await db + .select() + .from(embedding) + .where(and(eq(embedding.id, chunkId), eq(embedding.documentId, documentId))) + .limit(1) + + if (chunk.length === 0) { + return { hasAccess: false, notFound: true, reason: 'Chunk not found' } + } + + return { + hasAccess: true, + chunk: chunk[0] as EmbeddingData, + document: docData, + knowledgeBase: kbAccess.knowledgeBase!, + } +} diff --git a/apps/sim/app/api/mcp/servers/route.ts b/apps/sim/app/api/mcp/servers/route.ts index cfcc099eb85..ddf5e468a9f 100644 --- a/apps/sim/app/api/mcp/servers/route.ts +++ b/apps/sim/app/api/mcp/servers/route.ts @@ -134,10 +134,10 @@ export const POST = withRouteHandler( ) /** - * DELETE - Delete an MCP server from the workspace (requires admin permission) + * DELETE - Delete an MCP server from the workspace (requires write permission) */ export const DELETE = withRouteHandler( - withMcpAuth('admin')( + withMcpAuth('write')( async (request: NextRequest, { userId, userName, userEmail, workspaceId, requestId }) => { try { const { searchParams } = new URL(request.url) diff --git a/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts b/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts index 718b65aa0bb..d765c4985f7 100644 --- a/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts +++ b/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts @@ -147,7 +147,7 @@ export const PATCH = withRouteHandler( * DELETE - Delete a workflow MCP server and all its tools */ export const DELETE = withRouteHandler( - withMcpAuth('admin')( + withMcpAuth('write')( async ( request: NextRequest, { userId, userName, userEmail, workspaceId, requestId }, diff --git a/apps/sim/app/api/schedules/route.ts b/apps/sim/app/api/schedules/route.ts index 37bdc00ab24..ca36c978cef 100644 --- a/apps/sim/app/api/schedules/route.ts +++ b/apps/sim/app/api/schedules/route.ts @@ -220,8 +220,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => { const { workspaceId, title, prompt, cronExpression, timezone, lifecycle, maxRuns, startDate } = parsed.data.body - const hasPermission = await verifyWorkspaceMembership(session.user.id, workspaceId) - if (!hasPermission) { + const permission = await verifyWorkspaceMembership(session.user.id, workspaceId) + if (permission !== 'admin' && permission !== 'write') { return NextResponse.json({ error: 'Not authorized' }, { status: 403 }) } diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index d752a3e6dc5..c7f48ed36b3 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -397,7 +397,42 @@ describe('Workflow By ID API Route', () => { expect(data.error).toBe('Cannot delete the only workflow in the workspace') }) - it.concurrent('should deny deletion for non-admin users', async () => { + it('should allow user with write permission to delete workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + } + + mockGetSession({ user: { id: 'user-123' } }) + + mockGetWorkflowById.mockResolvedValue(mockWorkflow) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: mockWorkflow, + workspacePermission: 'write', + }) + + mockPerformDeleteWorkflow.mockResolvedValue({ success: true }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.success).toBe(true) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith( + expect.objectContaining({ workflowId: 'workflow-123', action: 'write' }) + ) + }) + + it.concurrent('should deny deletion for read-only users', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'other-user', @@ -411,9 +446,9 @@ describe('Workflow By ID API Route', () => { mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: false, status: 403, - message: 'Unauthorized: Access denied to admin this workflow', + message: 'Unauthorized: Access denied to write this workflow', workflow: mockWorkflow, - workspacePermission: null, + workspacePermission: 'read', }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -425,7 +460,7 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Unauthorized: Access denied to admin this workflow') + expect(data.error).toBe('Unauthorized: Access denied to write this workflow') }) }) diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 2f3f0ebe3ce..a69fffc2715 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -184,7 +184,7 @@ export const DELETE = withRouteHandler( const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, userId, - action: 'admin', + action: 'write', }) const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/sub-block-renderer.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/sub-block-renderer.tsx index 4a2c0240c55..79704b63d01 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/sub-block-renderer.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/sub-block-renderer.tsx @@ -1,6 +1,11 @@ 'use client' -import { useEffect, useRef } from 'react' +import { useCallback, useEffect, useRef } from 'react' +import { + buildToolSubBlockId, + resolveToolParamSync, +} from '@/lib/workflows/tool-input/synthetic-subblocks' +import { parseStoredToolInputValue } from '@/lib/workflows/tool-input/types' import { SubBlock } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/sub-block' import type { SubBlockConfig as BlockSubBlockConfig } from '@/blocks/types' import type { ActiveSearchTarget } from '@/stores/panel/editor/store' @@ -47,7 +52,7 @@ export function ToolSubBlockRenderer({ canonicalToggle, activeSearchTarget, }: ToolSubBlockRendererProps) { - const syntheticId = `${subBlockId}-tool-${toolIndex}-${effectiveParamId}` + const syntheticId = buildToolSubBlockId(subBlockId, toolIndex, effectiveParamId) const toolParamValue = toolParams?.[effectiveParamId] ?? '' const isObjectType = OBJECT_SUBBLOCK_TYPES.has(subBlock.type) @@ -55,6 +60,26 @@ export function ToolSubBlockRenderer({ const onParamChangeRef = useRef(onParamChange) onParamChangeRef.current = onParamChange + const pushParamValueToStore = useCallback( + (rawValue: string) => { + syncedRef.current = rawValue + if (isObjectType && rawValue) { + try { + const parsed = JSON.parse(rawValue) + if (typeof parsed === 'object' && parsed !== null) { + useSubBlockStore.getState().setValue(blockId, syntheticId, parsed) + return + } + } catch {} + } + useSubBlockStore.getState().setValue(blockId, syntheticId, rawValue) + }, + [blockId, syntheticId, isObjectType] + ) + + const pushParamValueToStoreRef = useRef(pushParamValueToStore) + pushParamValueToStoreRef.current = pushParamValueToStore + useEffect(() => { const unsub = useSubBlockStore.subscribe((state, prevState) => { const wfId = useWorkflowRegistry.getState().activeWorkflowId @@ -62,29 +87,29 @@ export function ToolSubBlockRenderer({ const newVal = state.workflowValues[wfId]?.[blockId]?.[syntheticId] const oldVal = prevState.workflowValues[wfId]?.[blockId]?.[syntheticId] if (newVal === oldVal) return - const stringified = - newVal == null ? '' : typeof newVal === 'string' ? newVal : JSON.stringify(newVal) - if (stringified === syncedRef.current) return - syncedRef.current = stringified - onParamChangeRef.current(toolIndex, effectiveParamId, stringified) + + const result = resolveToolParamSync(newVal, syncedRef.current) + if (result.action === 'noop') return + + if (result.action === 'reproject') { + const tools = parseStoredToolInputValue( + useSubBlockStore.getState().getValue(blockId, subBlockId) + ) + const sourceValue = tools[toolIndex]?.params?.[effectiveParamId] + pushParamValueToStoreRef.current(typeof sourceValue === 'string' ? sourceValue : '') + return + } + + syncedRef.current = result.value + onParamChangeRef.current(toolIndex, effectiveParamId, result.value) }) return unsub - }, [blockId, syntheticId, toolIndex, effectiveParamId]) + }, [blockId, subBlockId, syntheticId, toolIndex, effectiveParamId]) useEffect(() => { if (toolParamValue === syncedRef.current) return - syncedRef.current = toolParamValue - if (isObjectType && toolParamValue) { - try { - const parsed = JSON.parse(toolParamValue) - if (typeof parsed === 'object' && parsed !== null) { - useSubBlockStore.getState().setValue(blockId, syntheticId, parsed) - return - } - } catch {} - } - useSubBlockStore.getState().setValue(blockId, syntheticId, toolParamValue) - }, [toolParamValue, blockId, syntheticId, isObjectType]) + pushParamValueToStore(toolParamValue) + }, [toolParamValue, pushParamValueToStore]) const visibility = subBlock.paramVisibility ?? 'user-or-llm' const isOptionalForUser = visibility !== 'user-only' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index a1289ab653c..e7bfd632a3c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -27,6 +27,7 @@ import { import type { McpToolSchema } from '@/lib/mcp/types' import { getProviderIdFromServiceId, type OAuthProvider, type OAuthService } from '@/lib/oauth' import { extractInputFieldsFromBlocks } from '@/lib/workflows/input-format' +import { buildToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { McpServerFormModal } from '@/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal' import { @@ -1159,7 +1160,7 @@ export const ToolInput = memo(function ToolInput({ ) => { const uniqueSubBlockId = toolIndex !== undefined - ? `${subBlockId}-tool-${toolIndex}-${param.id}` + ? buildToolSubBlockId(subBlockId, toolIndex, param.id) : `${subBlockId}-${param.id}` const paramActiveSearchTarget = getParamActiveSearchTarget( toolIndex, @@ -2060,7 +2061,7 @@ export const ToolInput = memo(function ToolInput({ activeSearchTarget={getParamActiveSearchTarget( toolIndex, effectiveParamId, - `${subBlockId}-tool-${toolIndex}-${effectiveParamId}` + buildToolSubBlockId(subBlockId, toolIndex, effectiveParamId) )} /> ) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx index f69b72c4655..81606593445 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/folder-item/folder-item.tsx @@ -612,7 +612,7 @@ export function FolderItem({ !userPermissions.canEdit || isDuplicatingSelection || !hasExportableContent } disableExport={!userPermissions.canEdit || isExporting || !hasExportableContent} - showDelete={userPermissions.canAdmin} + showDelete={userPermissions.canEdit} disableDelete={effectiveLocked || !canDeleteSelection} onToggleLock={handleToggleLock} showLock={!isMixedSelection && selectedFolders.size <= 1} diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx index 991a69868e1..151eb90fabf 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx @@ -524,7 +524,7 @@ export function WorkflowItem({ disableDuplicate={!userPermissions.canEdit || isDuplicatingSelection} disableExport={!userPermissions.canEdit} disableColorChange={!userPermissions.canEdit || effectiveLocked} - showDelete={userPermissions.canAdmin} + showDelete={userPermissions.canEdit} disableDelete={!canDeleteSelection || effectiveLocked} onToggleLock={handleToggleLock} showLock={!isMixedSelection && selectedWorkflows.size <= 1} diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index a4b58338886..b745dc5fbe4 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -22,6 +22,7 @@ import { type WorkflowSearchSubflowFieldId, workflowSearchSubflowFieldMatchesExpected, } from '@/lib/workflows/search-replace/subflow-fields' +import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' import { useSocket } from '@/app/workspace/providers/socket-provider' import { getBlock } from '@/blocks' import { getSubBlocksDependingOnChange } from '@/blocks/utils' @@ -1494,7 +1495,7 @@ export function useCollaborativeWorkflow() { useSubBlockStore.getState().setValue(blockId, subblockId, value) useWorkflowStore.getState().syncDynamicHandleSubblockValue(blockId, subblockId, value) - if (activeWorkflowId) { + if (activeWorkflowId && !isSyntheticToolSubBlockId(subblockId)) { const operationId = generateId() addToQueue({ @@ -1604,6 +1605,10 @@ export function useCollaborativeWorkflow() { return false } + const persistedUpdates = updates.filter( + (update) => !isSyntheticToolSubBlockId(update.subblockId) + ) + if (updates.length > 0) { updates.forEach((update) => { useSubBlockStore.getState().setValue(update.blockId, update.subblockId, update.value) @@ -1612,21 +1617,23 @@ export function useCollaborativeWorkflow() { .syncDynamicHandleSubblockValue(update.blockId, update.subblockId, update.value) }) - const operationId = generateId() - addToQueue({ - id: operationId, - operation: { - operation: SUBBLOCK_OPERATIONS.BATCH_UPDATE, - target: OPERATION_TARGETS.SUBBLOCK, - payload: { updates }, - }, - workflowId: activeWorkflowId, - userId: session?.user?.id || 'unknown', - }) + if (persistedUpdates.length > 0) { + const operationId = generateId() + addToQueue({ + id: operationId, + operation: { + operation: SUBBLOCK_OPERATIONS.BATCH_UPDATE, + target: OPERATION_TARGETS.SUBBLOCK, + payload: { updates: persistedUpdates }, + }, + workflowId: activeWorkflowId, + userId: session?.user?.id || 'unknown', + }) + } } undoRedo.recordBatchUpdateSubblocks( - updates.map((update) => ({ + persistedUpdates.map((update) => ({ blockId: update.blockId, subBlockId: update.subblockId, before: update.expectedValue, @@ -1657,6 +1664,8 @@ export function useCollaborativeWorkflow() { // Apply locally first (immediate UI feedback) useSubBlockStore.getState().setValue(blockId, subblockId, value) + if (isSyntheticToolSubBlockId(subblockId)) return + // Use the operation queue but with immediate processing (no debouncing) const operationId = generateId() diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts index 9c450f56268..93b8a0899c2 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts @@ -990,7 +990,7 @@ export async function executeDeleteWorkflow( const { workflow: workflowRecord } = await ensureWorkflowAccess( workflowId, context.userId, - 'admin' + 'write' ) assertWorkflowMutationNotAborted(context) @@ -1025,7 +1025,7 @@ export async function executeDeleteFolder( } const workspaceId = context.workspaceId || (await getDefaultWorkspaceId(context.userId)) - await ensureWorkspaceAccess(workspaceId, context.userId, 'admin') + await ensureWorkspaceAccess(workspaceId, context.userId, 'write') const folders = await listFolders(workspaceId) const deleted: string[] = [] diff --git a/apps/sim/lib/uploads/server/metadata.ts b/apps/sim/lib/uploads/server/metadata.ts index f790413b979..9c2940e2d29 100644 --- a/apps/sim/lib/uploads/server/metadata.ts +++ b/apps/sim/lib/uploads/server/metadata.ts @@ -267,3 +267,45 @@ export async function deleteFileMetadata(key: string): Promise { .where(and(eq(workspaceFiles.key, key), isNull(workspaceFiles.deletedAt))) return true } + +/** + * Fields needed to record a trusted storage-key -> workspace ownership binding + * for a knowledge-base file. The `context` is always `'knowledge-base'`, so it is + * not part of this shape. + */ +export interface KnowledgeBaseFileOwnership { + key: string + userId: string + workspaceId: string + originalName: string + contentType: string + size: number +} + +/** + * Record the ownership binding for a single knowledge-base upload. KB file + * authorization (`verifyKBFileAccess`) resolves the owning workspace from this + * binding, so every KB object must have exactly one. Single source of truth for + * the binding shape across the presigned, batch-presigned, and multipart upload + * paths — keep all callers routed through here so they cannot drift. + */ +export async function recordKnowledgeBaseFileOwnership( + ownership: KnowledgeBaseFileOwnership +): Promise { + await insertFileMetadata({ ...ownership, context: 'knowledge-base' }) +} + +/** + * Bulk variant of {@link recordKnowledgeBaseFileOwnership} for batch upload flows. + * Idempotent against the active-key unique index (ON CONFLICT DO NOTHING). + */ +export async function recordKnowledgeBaseFileOwnershipMany( + ownerships: KnowledgeBaseFileOwnership[] +): Promise { + if (ownerships.length === 0) { + return + } + await insertFileMetadataMany( + ownerships.map((ownership) => ({ ...ownership, context: 'knowledge-base' })) + ) +} diff --git a/apps/sim/lib/workflows/comparison/compare.test.ts b/apps/sim/lib/workflows/comparison/compare.test.ts index 1921619f795..8b39f481be5 100644 --- a/apps/sim/lib/workflows/comparison/compare.test.ts +++ b/apps/sim/lib/workflows/comparison/compare.test.ts @@ -592,6 +592,104 @@ describe('hasWorkflowChanged', () => { }) }) + describe('Block-based tool params (synthetic projection)', () => { + const knowledgeTool = (kbId: string) => ({ + type: 'knowledge', + toolId: 'knowledge_search', + operation: 'search', + params: { knowledgeBaseSelector: kbId }, + usageControl: 'auto', + }) + + it.concurrent('ignores synthetic tool subblocks present only in the current state', () => { + const current = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { + tools: { value: [knowledgeTool('kb-123')] }, + 'tools-tool-0-knowledgeBaseSelector': { value: 'kb-123' }, + }, + }), + }, + }) + const deployed = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { tools: { value: [knowledgeTool('kb-123')] } }, + }), + }, + }) + expect(hasWorkflowChanged(current, deployed)).toBe(false) + }) + + it.concurrent('detects an actual change to a tool param value', () => { + const current = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { + tools: { value: [knowledgeTool('kb-999')] }, + 'tools-tool-0-knowledgeBaseSelector': { value: 'kb-999' }, + }, + }), + }, + }) + const deployed = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { tools: { value: [knowledgeTool('kb-123')] } }, + }), + }, + }) + expect(hasWorkflowChanged(current, deployed)).toBe(true) + }) + + it.concurrent('treats a cleared tool param as a change (deselect symptom)', () => { + const current = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { tools: { value: [knowledgeTool('')] } }, + }), + }, + }) + const deployed = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { tools: { value: [knowledgeTool('kb-123')] } }, + }), + }, + }) + expect(hasWorkflowChanged(current, deployed)).toBe(true) + }) + + it.concurrent('ignores synthetic subblocks for object-typed tool params', () => { + const fileTool = { + type: 'someservice', + toolId: 'someservice_upload', + operation: 'upload', + params: { file: '{"name":"a.pdf"}' }, + usageControl: 'auto', + } + const current = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { + tools: { value: [fileTool] }, + 'tools-tool-0-file': { value: { name: 'a.pdf' } }, + }, + }), + }, + }) + const deployed = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + subBlocks: { tools: { value: [fileTool] } }, + }), + }, + }) + expect(hasWorkflowChanged(current, deployed)).toBe(false) + }) + }) + describe('InputFormat SubBlock Special Handling', () => { it.concurrent('should ignore collapsed field but detect value changes in inputFormat', () => { // Only collapsed changes - should NOT detect as change diff --git a/apps/sim/lib/workflows/tool-input/synthetic-subblocks.test.ts b/apps/sim/lib/workflows/tool-input/synthetic-subblocks.test.ts new file mode 100644 index 00000000000..4597a0cd2e0 --- /dev/null +++ b/apps/sim/lib/workflows/tool-input/synthetic-subblocks.test.ts @@ -0,0 +1,59 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + buildToolSubBlockId, + isSyntheticToolSubBlockId, + resolveToolParamSync, +} from '@/lib/workflows/tool-input/synthetic-subblocks' + +describe('buildToolSubBlockId', () => { + it('composes the synthetic id from aggregate id, tool index, and param id', () => { + expect(buildToolSubBlockId('tools', 0, 'knowledgeBaseSelector')).toBe( + 'tools-tool-0-knowledgeBaseSelector' + ) + expect(buildToolSubBlockId('tools', 3, 'credential')).toBe('tools-tool-3-credential') + }) + + it('produces ids recognized by isSyntheticToolSubBlockId', () => { + expect(isSyntheticToolSubBlockId(buildToolSubBlockId('tools', 0, 'file'))).toBe(true) + expect(isSyntheticToolSubBlockId(buildToolSubBlockId('tools', 12, 'documentTags'))).toBe(true) + }) +}) + +describe('isSyntheticToolSubBlockId', () => { + it('returns false for real subblock ids', () => { + expect(isSyntheticToolSubBlockId('tools')).toBe(false) + expect(isSyntheticToolSubBlockId('model')).toBe(false) + expect(isSyntheticToolSubBlockId('knowledgeBaseSelector')).toBe(false) + expect(isSyntheticToolSubBlockId('tools-credential')).toBe(false) + }) +}) + +describe('resolveToolParamSync', () => { + it('re-projects a removed key instead of clearing params', () => { + expect(resolveToolParamSync(undefined, 'kb-123')).toEqual({ action: 'reproject' }) + }) + + it('mirrors a user clear as an empty string', () => { + expect(resolveToolParamSync('', 'kb-123')).toEqual({ action: 'mirror', value: '' }) + expect(resolveToolParamSync(null, 'kb-123')).toEqual({ action: 'mirror', value: '' }) + }) + + it('mirrors a changed value', () => { + expect(resolveToolParamSync('kb-456', 'kb-123')).toEqual({ action: 'mirror', value: 'kb-456' }) + }) + + it('is a noop when the value already matches the synced value', () => { + expect(resolveToolParamSync('kb-123', 'kb-123')).toEqual({ action: 'noop' }) + }) + + it('stringifies object values for object-typed params', () => { + expect(resolveToolParamSync({ name: 'a.pdf' }, '{"name":"a.pdf"}')).toEqual({ action: 'noop' }) + expect(resolveToolParamSync({ name: 'b.pdf' }, '{"name":"a.pdf"}')).toEqual({ + action: 'mirror', + value: '{"name":"b.pdf"}', + }) + }) +}) diff --git a/apps/sim/lib/workflows/tool-input/synthetic-subblocks.ts b/apps/sim/lib/workflows/tool-input/synthetic-subblocks.ts index 35686d79b43..59d948a36c6 100644 --- a/apps/sim/lib/workflows/tool-input/synthetic-subblocks.ts +++ b/apps/sim/lib/workflows/tool-input/synthetic-subblocks.ts @@ -1,11 +1,60 @@ -const SYNTHETIC_TOOL_SUBBLOCK_RE = /-tool-\d+-/ +const TOOL_SUBBLOCK_INFIX = '-tool-' +const SYNTHETIC_TOOL_SUBBLOCK_RE = new RegExp(`${TOOL_SUBBLOCK_INFIX}\\d+-`) /** - * Returns true for ToolSubBlockRenderer mirror subblocks. + * Builds the synthetic subblock id used to render a block-based tool's param + * inside a tool row. * - * These IDs follow `{subBlockId}-tool-{index}-{paramId}` and duplicate values - * already stored in the aggregate `tool-input` subblock. + * The id follows `{subBlockId}-tool-{index}-{paramId}` and is an ephemeral, + * client-only projection key for the value held canonically at + * `tool.params[paramId]` in the aggregate `tool-input` subblock. + */ +export function buildToolSubBlockId( + aggregateSubBlockId: string, + toolIndex: number, + paramId: string +): string { + return `${aggregateSubBlockId}${TOOL_SUBBLOCK_INFIX}${toolIndex}-${paramId}` +} + +/** + * Returns true for ToolSubBlockRenderer mirror subblocks produced by + * {@link buildToolSubBlockId}. These duplicate values already stored in the + * aggregate `tool-input` subblock and must never be persisted or compared. */ export function isSyntheticToolSubBlockId(subBlockId: string): boolean { return SYNTHETIC_TOOL_SUBBLOCK_RE.test(subBlockId) } + +type ToolParamSyncAction = + | { action: 'noop' } + | { action: 'reproject' } + | { action: 'mirror'; value: string } + +/** + * Reconciles a change to a synthetic tool subblock value with the canonical + * `tool.params`. The synthetic key is a projection of `tool.params`, so: + * + * - `undefined` means the key was dropped by a wholesale store replace — restore + * it from `tool.params` ({@link ToolParamSyncAction.action} `reproject`). A + * removal is never a user clear. + * - any defined value (including `''` or `null`, both meaning "cleared by the + * user") is a genuine edit to write back (`mirror`). + * - a value already matching `syncedValue` needs no work (`noop`). + */ +export function resolveToolParamSync( + storeValue: unknown, + syncedValue: string | null +): ToolParamSyncAction { + if (storeValue === undefined) return { action: 'reproject' } + + const stringified = + storeValue === null + ? '' + : typeof storeValue === 'string' + ? storeValue + : JSON.stringify(storeValue) + + if (stringified === syncedValue) return { action: 'noop' } + return { action: 'mirror', value: stringified } +}