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

* fix(condition): used isolated vms for condition block RCE * ack PR comment * one more * remove inputForm from sched, update loop condition to also use isolated vm * hide servicenow
2026-04-06 03:00:16 -04:00 · 2025-12-17 15:29:25 -08:00
parent 7b5405e968
commit c4a6d11cc0
7 changed files with 154 additions and 64 deletions
--- a/apps/sim/blocks/blocks/schedule.ts
+++ b/apps/sim/blocks/blocks/schedule.ts
@@ -155,15 +155,6 @@ export const ScheduleBlock: BlockConfig = {
      condition: { field: 'scheduleType', value: ['minutes', 'hourly'], not: true },
    },

-    {
-      id: 'inputFormat',
-      title: 'Input Format',
-      type: 'input-format',
-      description:
-        'Define input parameters that will be available when the schedule triggers. Use Value to set default values for scheduled executions.',
-      mode: 'trigger',
-    },
-
    {
      id: 'scheduleSave',
      type: 'schedule-save',
--- a/apps/sim/blocks/blocks/servicenow.ts
+++ b/apps/sim/blocks/blocks/servicenow.ts
@@ -8,6 +8,7 @@ export const ServiceNowBlock: BlockConfig<ServiceNowResponse> = {
  name: 'ServiceNow',
  description: 'Create, read, update, delete, and bulk import ServiceNow records',
  authMode: AuthMode.OAuth,
+  hideFromToolbar: true,
  longDescription:
    'Integrate ServiceNow into your workflow. Can create, read, update, and delete records in any ServiceNow table (incidents, tasks, users, etc.). Supports bulk import operations for data migration and ETL.',
  docsLink: 'https://docs.sim.ai/tools/servicenow',
--- a/apps/sim/executor/handlers/condition/condition-handler.test.ts
+++ b/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,7 +344,6 @@ describe('ConditionBlockHandler', () => {
    ]
    const inputs = { conditions: JSON.stringify(conditions) }

-    // Mock the full resolution pipeline
    mockResolver.resolveVariableReferences.mockReturnValue(
      'context.nonExistentProperty.doSomething()'
    )
@@ -325,7 +351,7 @@ describe('ConditionBlockHandler', () => {
    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/
    )
  })

@@ -333,7 +359,6 @@ describe('ConditionBlockHandler', () => {
    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"')
--- a/apps/sim/executor/handlers/condition/condition-handler.ts
+++ b/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: { 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 }
  }
--- a/apps/sim/executor/orchestrators/loop.ts
+++ b/apps/sim/executor/orchestrators/loop.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 { buildLoopIndexCondition, DEFAULTS, EDGE } from '@/executor/constants'
 import type { DAG } from '@/executor/dag/builder'
@@ -17,6 +19,8 @@ import type { SerializedLoop } from '@/serializer/types'

 const logger = createLogger('LoopOrchestrator')

+const LOOP_CONDITION_TIMEOUT_MS = 5000
+
 export type LoopRoute = typeof EDGE.LOOP_CONTINUE | typeof EDGE.LOOP_EXIT

 export interface LoopContinuationResult {
@@ -112,7 +116,10 @@ export class LoopOrchestrator {
    scope.currentIterationOutputs.set(baseId, output)
  }

-  evaluateLoopContinuation(ctx: ExecutionContext, loopId: string): LoopContinuationResult {
+  async evaluateLoopContinuation(
+    ctx: ExecutionContext,
+    loopId: string
+  ): Promise<LoopContinuationResult> {
    const scope = ctx.loopExecutions?.get(loopId)
    if (!scope) {
      logger.error('Loop scope not found during continuation evaluation', { loopId })
@@ -123,7 +130,6 @@ export class LoopOrchestrator {
      }
    }

-    // Check for cancellation
    if (ctx.isCancelled) {
      logger.info('Loop execution cancelled', { loopId, iteration: scope.iteration })
      return this.createExitResult(ctx, loopId, scope)
@@ -140,7 +146,7 @@ export class LoopOrchestrator {

    scope.currentIterationOutputs.clear()

-    if (!this.evaluateCondition(ctx, scope, scope.iteration + 1)) {
+    if (!(await this.evaluateCondition(ctx, scope, scope.iteration + 1))) {
      return this.createExitResult(ctx, loopId, scope)
    }

@@ -173,7 +179,11 @@ export class LoopOrchestrator {
    }
  }

-  private evaluateCondition(ctx: ExecutionContext, scope: LoopScope, iteration?: number): boolean {
+  private async evaluateCondition(
+    ctx: ExecutionContext,
+    scope: LoopScope,
+    iteration?: number
+  ): Promise<boolean> {
    if (!scope.condition) {
      logger.warn('No condition defined for loop')
      return false
@@ -184,7 +194,7 @@ export class LoopOrchestrator {
      scope.iteration = iteration
    }

-    const result = this.evaluateWhileCondition(ctx, scope.condition, scope)
+    const result = await this.evaluateWhileCondition(ctx, scope.condition, scope)

    if (iteration !== undefined) {
      scope.iteration = currentIteration
@@ -223,7 +233,6 @@ export class LoopOrchestrator {
    const loopNodes = loopConfig.nodes
    const allLoopNodeIds = new Set([sentinelStartId, sentinelEndId, ...loopNodes])

-    // Clear deactivated edges for loop nodes so error/success edges can be re-evaluated
    if (this.edgeManager) {
      this.edgeManager.clearDeactivatedEdgesForNodes(allLoopNodeIds)
    }
@@ -263,7 +272,7 @@ export class LoopOrchestrator {
   *
   * @returns true if the loop should execute, false if it should be skipped
   */
-  evaluateInitialCondition(ctx: ExecutionContext, loopId: string): boolean {
+  async evaluateInitialCondition(ctx: ExecutionContext, loopId: string): Promise<boolean> {
    const scope = ctx.loopExecutions?.get(loopId)
    if (!scope) {
      logger.warn('Loop scope not found for initial condition evaluation', { loopId })
@@ -300,7 +309,7 @@ export class LoopOrchestrator {
        return false
      }

-      const result = this.evaluateWhileCondition(ctx, scope.condition, scope)
+      const result = await this.evaluateWhileCondition(ctx, scope.condition, scope)
      logger.info('While loop initial condition evaluation', {
        loopId,
        condition: scope.condition,
@@ -327,11 +336,11 @@ export class LoopOrchestrator {
    return undefined
  }

-  private evaluateWhileCondition(
+  private async evaluateWhileCondition(
    ctx: ExecutionContext,
    condition: string,
    scope: LoopScope
-  ): boolean {
+  ): Promise<boolean> {
    if (!condition) {
      return false
    }
@@ -343,7 +352,6 @@ export class LoopOrchestrator {
        workflowVariables: ctx.workflowVariables,
      })

-      // Use generic utility for smart variable reference replacement
      const evaluatedCondition = replaceValidReferences(condition, (match) => {
        const resolved = this.resolver.resolveSingleReference(ctx, '', match, scope)
        logger.info('Resolved variable reference in loop condition', {
@@ -352,11 +360,9 @@ export class LoopOrchestrator {
          resolvedType: typeof resolved,
        })
        if (resolved !== undefined) {
-          // For booleans and numbers, return as-is (no quotes)
          if (typeof resolved === 'boolean' || typeof resolved === 'number') {
            return String(resolved)
          }
-          // For strings that represent booleans, return without quotes
          if (typeof resolved === 'string') {
            const lower = resolved.toLowerCase().trim()
            if (lower === 'true' || lower === 'false') {
@@ -364,13 +370,33 @@ export class LoopOrchestrator {
            }
            return `"${resolved}"`
          }
-          // For other types, stringify them
          return JSON.stringify(resolved)
        }
        return match
      })

-      const result = Boolean(new Function(`return (${evaluatedCondition})`)())
+      const requestId = generateRequestId()
+      const code = `return Boolean(${evaluatedCondition})`
+
+      const vmResult = await executeInIsolatedVM({
+        code,
+        params: {},
+        envVars: {},
+        contextVariables: {},
+        timeoutMs: LOOP_CONDITION_TIMEOUT_MS,
+        requestId,
+      })
+
+      if (vmResult.error) {
+        logger.error('Failed to evaluate loop condition', {
+          condition,
+          evaluatedCondition,
+          error: vmResult.error,
+        })
+        return false
+      }
+
+      const result = Boolean(vmResult.result)

      logger.info('Loop condition evaluation result', {
        originalCondition: condition,
--- a/apps/sim/executor/orchestrators/node.ts
+++ b/apps/sim/executor/orchestrators/node.ts
@@ -68,7 +68,7 @@ export class NodeExecutionOrchestrator {
    }

    if (node.metadata.isSentinel) {
-      const output = this.handleSentinel(ctx, node)
+      const output = await this.handleSentinel(ctx, node)
      const isFinalOutput = node.outgoingEdges.size === 0
      return {
        nodeId,
@@ -86,14 +86,17 @@ export class NodeExecutionOrchestrator {
    }
  }

-  private handleSentinel(ctx: ExecutionContext, node: DAGNode): NormalizedBlockOutput {
+  private async handleSentinel(
+    ctx: ExecutionContext,
+    node: DAGNode
+  ): Promise<NormalizedBlockOutput> {
    const sentinelType = node.metadata.sentinelType
    const loopId = node.metadata.loopId

    switch (sentinelType) {
      case 'start': {
        if (loopId) {
-          const shouldExecute = this.loopOrchestrator.evaluateInitialCondition(ctx, loopId)
+          const shouldExecute = await this.loopOrchestrator.evaluateInitialCondition(ctx, loopId)
          if (!shouldExecute) {
            logger.info('While loop initial condition false, skipping loop body', { loopId })
            return {
@@ -112,7 +115,7 @@ export class NodeExecutionOrchestrator {
          return { shouldExit: true, selectedRoute: EDGE.LOOP_EXIT }
        }

-        const continuationResult = this.loopOrchestrator.evaluateLoopContinuation(ctx, loopId)
+        const continuationResult = await this.loopOrchestrator.evaluateLoopContinuation(ctx, loopId)

        if (continuationResult.shouldContinue) {
          return {
--- a/apps/sim/lib/execution/isolated-vm.ts
+++ b/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)
        }