Skip to content

Commit 3664a56

Browse files
waleedlatif1claude
andcommitted
fix(socket): add server-side lock validation and admin-only permissions
1. BATCH_TOGGLE_LOCKED now requires admin role - non-admin users with write role can no longer bypass UI restriction via direct socket messages 2. BATCH_REMOVE_BLOCKS now validates lock status server-side - filters out protected blocks (locked or inside locked parent) before deletion 3. Remove duplicate/outdated comment in regenerateBlockIds Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent ef4acfd commit 3664a56

File tree

3 files changed

+64
-33
lines changed

3 files changed

+64
-33
lines changed

apps/sim/socket/database/operations.ts

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -625,44 +625,74 @@ async function handleBlocksOperationTx(
625625

626626
logger.info(`Batch removing ${ids.length} blocks from workflow ${workflowId}`)
627627

628-
// Collect all block IDs including children of subflows
629-
const allBlocksToDelete = new Set<string>(ids)
628+
// Fetch all blocks to check lock status and filter out protected blocks
629+
const allBlocks = await tx
630+
.select({
631+
id: workflowBlocks.id,
632+
type: workflowBlocks.type,
633+
locked: workflowBlocks.locked,
634+
data: workflowBlocks.data,
635+
})
636+
.from(workflowBlocks)
637+
.where(eq(workflowBlocks.workflowId, workflowId))
630638

631-
for (const id of ids) {
632-
const blockToRemove = await tx
633-
.select({ type: workflowBlocks.type })
634-
.from(workflowBlocks)
635-
.where(and(eq(workflowBlocks.id, id), eq(workflowBlocks.workflowId, workflowId)))
636-
.limit(1)
639+
type BlockRecord = (typeof allBlocks)[number]
640+
const blocksById: Record<string, BlockRecord> = Object.fromEntries(
641+
allBlocks.map((b: BlockRecord) => [b.id, b])
642+
)
637643

638-
if (blockToRemove.length > 0 && isSubflowBlockType(blockToRemove[0].type)) {
639-
const childBlocks = await tx
640-
.select({ id: workflowBlocks.id })
641-
.from(workflowBlocks)
642-
.where(
643-
and(
644-
eq(workflowBlocks.workflowId, workflowId),
645-
sql`${workflowBlocks.data}->>'parentId' = ${id}`
646-
)
647-
)
644+
// Helper to check if a block is protected (locked or inside locked parent)
645+
const isProtected = (blockId: string): boolean => {
646+
const block = blocksById[blockId]
647+
if (!block) return false
648+
if (block.locked) return true
649+
const parentId = (block.data as Record<string, unknown> | null)?.parentId as
650+
| string
651+
| undefined
652+
if (parentId && blocksById[parentId]?.locked) return true
653+
return false
654+
}
648655

649-
childBlocks.forEach((child: { id: string }) => allBlocksToDelete.add(child.id))
656+
// Filter out protected blocks from deletion request
657+
const deletableIds = ids.filter((id) => !isProtected(id))
658+
if (deletableIds.length === 0) {
659+
logger.info('All requested blocks are protected, skipping deletion')
660+
return
661+
}
662+
663+
if (deletableIds.length < ids.length) {
664+
logger.info(
665+
`Filtered out ${ids.length - deletableIds.length} protected blocks from deletion`
666+
)
667+
}
668+
669+
// Collect all block IDs including children of subflows
670+
const allBlocksToDelete = new Set<string>(deletableIds)
671+
672+
for (const id of deletableIds) {
673+
const block = blocksById[id]
674+
if (block && isSubflowBlockType(block.type)) {
675+
// Include all children of the subflow (they should be deleted with parent)
676+
for (const b of allBlocks) {
677+
const parentId = (b.data as Record<string, unknown> | null)?.parentId
678+
if (parentId === id) {
679+
allBlocksToDelete.add(b.id)
680+
}
681+
}
650682
}
651683
}
652684

653685
const blockIdsArray = Array.from(allBlocksToDelete)
654686

655-
// Collect parent IDs BEFORE deleting blocks
687+
// Collect parent IDs BEFORE deleting blocks (use blocksById, already fetched)
656688
const parentIds = new Set<string>()
657-
for (const id of ids) {
658-
const parentInfo = await tx
659-
.select({ parentId: sql<string | null>`${workflowBlocks.data}->>'parentId'` })
660-
.from(workflowBlocks)
661-
.where(and(eq(workflowBlocks.id, id), eq(workflowBlocks.workflowId, workflowId)))
662-
.limit(1)
663-
664-
if (parentInfo.length > 0 && parentInfo[0].parentId) {
665-
parentIds.add(parentInfo[0].parentId)
689+
for (const id of deletableIds) {
690+
const block = blocksById[id]
691+
const parentId = (block?.data as Record<string, unknown> | null)?.parentId as
692+
| string
693+
| undefined
694+
if (parentId) {
695+
parentIds.add(parentId)
666696
}
667697
}
668698

apps/sim/socket/middleware/permissions.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import {
1414

1515
const logger = createLogger('SocketPermissions')
1616

17-
// All write operations (admin and write roles have same permissions)
17+
// Admin-only operations (require admin role)
18+
const ADMIN_ONLY_OPERATIONS: string[] = [BLOCKS_OPERATIONS.BATCH_TOGGLE_LOCKED]
19+
20+
// Write operations (admin and write roles both have these permissions)
1821
const WRITE_OPERATIONS: string[] = [
1922
// Block operations
2023
BLOCK_OPERATIONS.UPDATE_POSITION,
@@ -30,7 +33,6 @@ const WRITE_OPERATIONS: string[] = [
3033
BLOCKS_OPERATIONS.BATCH_REMOVE_BLOCKS,
3134
BLOCKS_OPERATIONS.BATCH_TOGGLE_ENABLED,
3235
BLOCKS_OPERATIONS.BATCH_TOGGLE_HANDLES,
33-
BLOCKS_OPERATIONS.BATCH_TOGGLE_LOCKED,
3436
BLOCKS_OPERATIONS.BATCH_UPDATE_PARENT,
3537
// Edge operations
3638
EDGE_OPERATIONS.ADD,
@@ -52,7 +54,7 @@ const READ_OPERATIONS: string[] = [
5254

5355
// Define operation permissions based on role
5456
const ROLE_PERMISSIONS: Record<string, string[]> = {
55-
admin: WRITE_OPERATIONS,
57+
admin: [...ADMIN_ONLY_OPERATIONS, ...WRITE_OPERATIONS],
5658
write: WRITE_OPERATIONS,
5759
read: READ_OPERATIONS,
5860
}

apps/sim/stores/workflows/utils.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,6 @@ export function regenerateBlockIds(
520520
}
521521
} else {
522522
// Parent doesn't exist anywhere OR parent is locked - clear the relationship
523-
// Parent doesn't exist anywhere - clear the relationship
524523
block.data = { ...block.data, parentId: undefined, extent: undefined }
525524
}
526525
}

0 commit comments

Comments
 (0)