fix(condition): used isolated vms for condition block RCE

Author: waleedlatif1

apps/sim/executor/handlers/condition/condition-handler.test.ts

@@ -1,11 +1,47 @@
-import '@/executor/__test-utils__/mock-dependencies'
-
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { BlockType } from '@/executor/constants'
 import { ConditionBlockHandler } from '@/executor/handlers/condition/condition-handler'
 import type { BlockState, ExecutionContext } from '@/executor/types'
 import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
 
+vi.mock('@/lib/logs/console/logger', () => ({
+  createLogger: vi.fn(() => ({
+    info: vi.fn(),
+    error: vi.fn(),
+    warn: vi.fn(),
+    debug: vi.fn(),
+  })),
+}))
+
+vi.mock('@/lib/core/utils/request', () => ({
+  generateRequestId: vi.fn(() => 'test-request-id'),
+}))
+
+vi.mock('@/lib/execution/isolated-vm', () => ({
+  executeInIsolatedVM: vi.fn(),
+}))
+
+import { executeInIsolatedVM } from '@/lib/execution/isolated-vm'
+
+const mockExecuteInIsolatedVM = executeInIsolatedVM as ReturnType<typeof vi.fn>
+
+function simulateIsolatedVMExecution(
+  code: string,
+  contextVariables: Record<string, unknown>
+): { result: unknown; stdout: string; error?: { message: string; name: string } } {
+  try {
+    const fn = new Function(...Object.keys(contextVariables), code)
+    const result = fn(...Object.values(contextVariables))
+    return { result, stdout: '' }
+  } catch (error: any) {
+    return {
+      result: null,
+      stdout: '',
+      error: { message: error.message, name: error.name || 'Error' },
+    }
+  }
+}
+
 describe('ConditionBlockHandler', () => {
   let handler: ConditionBlockHandler
   let mockBlock: SerializedBlock
@@ -18,7 +54,6 @@ describe('ConditionBlockHandler', () => {
   let mockPathTracker: any
 
   beforeEach(() => {
-    // Define blocks first
     mockSourceBlock = {
       id: 'source-block-1',
       metadata: { id: 'source', name: 'Source Block' },
@@ -33,7 +68,7 @@ describe('ConditionBlockHandler', () => {
       metadata: { id: BlockType.CONDITION, name: 'Test Condition' },
       position: { x: 50, y: 50 },
       config: { tool: BlockType.CONDITION, params: {} },
-      inputs: { conditions: 'json' }, // Corrected based on previous step
+      inputs: { conditions: 'json' },
       outputs: {},
       enabled: true,
     }
@@ -56,7 +91,6 @@ describe('ConditionBlockHandler', () => {
       enabled: true,
     }
 
-    // Then define workflow using the block objects
     mockWorkflow = {
       blocks: [mockSourceBlock, mockBlock, mockTargetBlock1, mockTargetBlock2],
       connections: [
@@ -84,7 +118,6 @@ describe('ConditionBlockHandler', () => {
 
     handler = new ConditionBlockHandler(mockPathTracker, mockResolver)
 
-    // Define mock context *after* workflow and blocks are set up
     mockContext = {
       workflowId: 'test-workflow-id',
       blockStates: new Map<string, BlockState>([
@@ -99,7 +132,7 @@ describe('ConditionBlockHandler', () => {
       ]),
       blockLogs: [],
       metadata: { duration: 0 },
-      environmentVariables: {}, // Now set the context's env vars
+      environmentVariables: {},
       decisions: { router: new Map(), condition: new Map() },
       loopExecutions: new Map(),
       executedBlocks: new Set([mockSourceBlock.id]),
@@ -108,11 +141,11 @@ describe('ConditionBlockHandler', () => {
       completedLoops: new Set(),
     }
 
-    // Reset mocks using vi
     vi.clearAllMocks()
 
-    // Default mock implementations - Removed as it's in the shared mock now
-    // mockResolver.resolveBlockReferences.mockImplementation((value) => value)
+    mockExecuteInIsolatedVM.mockImplementation(async ({ code, contextVariables }) => {
+      return simulateIsolatedVMExecution(code, contextVariables)
+    })
   })
 
   it('should handle condition blocks', () => {
@@ -141,7 +174,6 @@ describe('ConditionBlockHandler', () => {
       selectedOption: 'cond1',
     }
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue('context.value > 5')
     mockResolver.resolveBlockReferences.mockReturnValue('context.value > 5')
     mockResolver.resolveEnvVariables.mockReturnValue('context.value > 5')
@@ -182,7 +214,6 @@ describe('ConditionBlockHandler', () => {
       selectedOption: 'else1',
     }
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue('context.value < 0')
     mockResolver.resolveBlockReferences.mockReturnValue('context.value < 0')
     mockResolver.resolveEnvVariables.mockReturnValue('context.value < 0')
@@ -207,7 +238,7 @@ describe('ConditionBlockHandler', () => {
     const inputs = { conditions: '{ "invalid json ' }
 
     await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
-      /^Invalid conditions format: Unterminated string.*/
+      /^Invalid conditions format:/
     )
   })
 
@@ -218,7 +249,6 @@ describe('ConditionBlockHandler', () => {
     ]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue('{{source-block-1.value}} > 5')
     mockResolver.resolveBlockReferences.mockReturnValue('10 > 5')
     mockResolver.resolveEnvVariables.mockReturnValue('10 > 5')
@@ -245,7 +275,6 @@ describe('ConditionBlockHandler', () => {
     ]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Mock the full resolution pipeline for variable resolution
     mockResolver.resolveVariableReferences.mockReturnValue('"john" !== null')
     mockResolver.resolveBlockReferences.mockReturnValue('"john" !== null')
     mockResolver.resolveEnvVariables.mockReturnValue('"john" !== null')
@@ -272,7 +301,6 @@ describe('ConditionBlockHandler', () => {
     ]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Mock the full resolution pipeline for env variable resolution
     mockResolver.resolveVariableReferences.mockReturnValue('{{POOP}} === "hi"')
     mockResolver.resolveBlockReferences.mockReturnValue('{{POOP}} === "hi"')
     mockResolver.resolveEnvVariables.mockReturnValue('"hi" === "hi"')
@@ -300,7 +328,6 @@ describe('ConditionBlockHandler', () => {
     const inputs = { conditions: JSON.stringify(conditions) }
 
     const resolutionError = new Error('Could not resolve reference: invalid-ref')
-    // Mock the pipeline to throw at the variable resolution stage
     mockResolver.resolveVariableReferences.mockImplementation(() => {
       throw resolutionError
     })
@@ -317,23 +344,21 @@ describe('ConditionBlockHandler', () => {
     ]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue(
       'context.nonExistentProperty.doSomething()'
     )
     mockResolver.resolveBlockReferences.mockReturnValue('context.nonExistentProperty.doSomething()')
     mockResolver.resolveEnvVariables.mockReturnValue('context.nonExistentProperty.doSomething()')
 
     await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
-      /^Evaluation error in condition "if": Evaluation error in condition: Cannot read properties of undefined \(reading 'doSomething'\)\. \(Resolved: context\.nonExistentProperty\.doSomething\(\)\)$/
+      /Evaluation error in condition "if".*doSomething/
     )
   })
 
   it('should handle missing source block output gracefully', async () => {
     const conditions = [{ id: 'cond1', title: 'if', value: 'true' }]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Create a new context with empty blockStates instead of trying to delete from readonly map
     const contextWithoutSource = {
       ...mockContext,
       blockStates: new Map<string, BlockState>(),
@@ -355,7 +380,6 @@ describe('ConditionBlockHandler', () => {
 
     mockContext.workflow!.blocks = [mockSourceBlock, mockBlock, mockTargetBlock2]
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue('true')
     mockResolver.resolveBlockReferences.mockReturnValue('true')
     mockResolver.resolveEnvVariables.mockReturnValue('true')
@@ -381,7 +405,6 @@ describe('ConditionBlockHandler', () => {
       },
     ]
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences
       .mockReturnValueOnce('false')
       .mockReturnValueOnce('context.value === 99')
@@ -394,12 +417,10 @@ describe('ConditionBlockHandler', () => {
 
     const result = await handler.execute(mockContext, mockBlock, inputs)
 
-    // Should return success with no path selected (branch ends gracefully)
     expect((result as any).conditionResult).toBe(false)
     expect((result as any).selectedPath).toBeNull()
     expect((result as any).selectedConditionId).toBeNull()
     expect((result as any).selectedOption).toBeNull()
-    // Decision should not be set when no condition matches
     expect(mockContext.decisions.condition.has(mockBlock.id)).toBe(false)
   })
 
@@ -410,7 +431,6 @@ describe('ConditionBlockHandler', () => {
     ]
     const inputs = { conditions: JSON.stringify(conditions) }
 
-    // Mock the full resolution pipeline
     mockResolver.resolveVariableReferences.mockReturnValue('context.item === "apple"')
     mockResolver.resolveBlockReferences.mockReturnValue('context.item === "apple"')
     mockResolver.resolveEnvVariables.mockReturnValue('context.item === "apple"')

apps/sim/executor/handlers/condition/condition-handler.ts

@@ -1,3 +1,5 @@
+import { generateRequestId } from '@/lib/core/utils/request'
+import { executeInIsolatedVM } from '@/lib/execution/isolated-vm'
 import { createLogger } from '@/lib/logs/console/logger'
 import type { BlockOutput } from '@/blocks/types'
 import { BlockType, CONDITION, DEFAULTS, EDGE } from '@/executor/constants'
@@ -6,6 +8,8 @@ import type { SerializedBlock } from '@/serializer/types'
 
 const logger = createLogger('ConditionBlockHandler')
 
+const CONDITION_TIMEOUT_MS = 5000
+
 /**
  * Evaluates a single condition expression with variable/block reference resolution
  * Returns true if condition is met, false otherwise
@@ -35,11 +39,32 @@ export async function evaluateConditionExpression(
   }
 
   try {
-    const conditionMet = new Function(
-      'context',
-      `with(context) { return ${resolvedConditionValue} }`
-    )(evalContext)
-    return Boolean(conditionMet)
+    const requestId = generateRequestId()
+
+    const code = `return Boolean(${resolvedConditionValue})`
+
+    const result = await executeInIsolatedVM({
+      code,
+      params: {},
+      envVars: {},
+      contextVariables: { ...evalContext, context: evalContext },
+      timeoutMs: CONDITION_TIMEOUT_MS,
+      requestId,
+    })
+
+    if (result.error) {
+      logger.error(`Failed to evaluate condition: ${result.error.message}`, {
+        originalCondition: conditionExpression,
+        resolvedCondition: resolvedConditionValue,
+        evalContext,
+        error: result.error,
+      })
+      throw new Error(
+        `Evaluation error in condition: ${result.error.message}. (Resolved: ${resolvedConditionValue})`
+      )
+    }
+
+    return Boolean(result.result)
   } catch (evalError: any) {
     logger.error(`Failed to evaluate condition: ${evalError.message}`, {
       originalCondition: conditionExpression,
@@ -87,7 +112,6 @@ export class ConditionBlockHandler implements BlockHandler {
       block
     )
 
-    // Handle case where no condition matched and no else exists - branch ends gracefully
     if (!selectedConnection || !selectedCondition) {
       return {
         ...((sourceOutput as any) || {}),
@@ -206,14 +230,12 @@ export class ConditionBlockHandler implements BlockHandler {
       if (elseConnection) {
         return { selectedConnection: elseConnection, selectedCondition: elseCondition }
       }
-      // Else exists but has no connection - treat as no match, branch ends
       logger.info(`No condition matched and else has no connection - branch ending`, {
         blockId: block.id,
       })
       return { selectedConnection: null, selectedCondition: null }
     }
 
-    // No condition matched and no else exists - branch ends gracefully
     logger.info(`No condition matched and no else block - branch ending`, { blockId: block.id })
     return { selectedConnection: null, selectedCondition: null }
   }

apps/sim/lib/execution/isolated-vm.ts

@@ -204,12 +204,17 @@ async function ensureWorker(): Promise<void> {
 
     import('node:child_process').then(({ spawn }) => {
       worker = spawn('node', [workerPath], {
-        stdio: ['ignore', 'pipe', 'inherit', 'ipc'],
+        stdio: ['ignore', 'pipe', 'pipe', 'ipc'],
         serialization: 'json',
       })
 
       worker.on('message', handleWorkerMessage)
 
+      let stderrData = ''
+      worker.stderr?.on('data', (data: Buffer) => {
+        stderrData += data.toString()
+      })
+
       const startTimeout = setTimeout(() => {
         worker?.kill()
         worker = null
@@ -232,20 +237,42 @@ async function ensureWorker(): Promise<void> {
       }
       worker.on('message', readyHandler)
 
-      worker.on('exit', () => {
+      worker.on('exit', (code) => {
         if (workerIdleTimeout) {
           clearTimeout(workerIdleTimeout)
           workerIdleTimeout = null
         }
+
+        const wasStartupFailure = !workerReady && workerReadyPromise
+
         worker = null
         workerReady = false
         workerReadyPromise = null
+
+        let errorMessage = 'Worker process exited unexpectedly'
+        if (stderrData.includes('isolated_vm') || stderrData.includes('MODULE_NOT_FOUND')) {
+          errorMessage =
+            'Code execution requires the isolated-vm native module which failed to load. ' +
+            'This usually means the module needs to be rebuilt for your Node.js version. ' +
+            'Please run: cd node_modules/isolated-vm && npm rebuild'
+          logger.error('isolated-vm module failed to load', { stderr: stderrData })
+        } else if (stderrData) {
+          errorMessage = `Worker process failed: ${stderrData.slice(0, 500)}`
+          logger.error('Worker process failed', { stderr: stderrData })
+        }
+
+        if (wasStartupFailure) {
+          clearTimeout(startTimeout)
+          reject(new Error(errorMessage))
+          return
+        }
+
         for (const [id, pending] of pendingExecutions) {
           clearTimeout(pending.timeout)
           pending.resolve({
             result: null,
             stdout: '',
-            error: { message: 'Worker process exited unexpectedly', name: 'WorkerError' },
+            error: { message: errorMessage, name: 'WorkerError' },
           })
           pendingExecutions.delete(id)
         }