Skip to content

Commit d3d686b

Browse files
committed
fix(executor): escape resolved references in while/doWhile loop conditions
Loop conditions inlined each resolved reference as a bare double-quoted literal before compiling the expression in the execution isolate, so a quote anywhere in the referenced value broke out of the literal and ran as code. Serialize operands as proper JS literals instead, and cap condition-driven loops, which had no iteration ceiling at all.
1 parent 9064039 commit d3d686b

2 files changed

Lines changed: 153 additions & 12 deletions

File tree

apps/sim/executor/orchestrators/loop.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,98 @@ describe('LoopOrchestrator', () => {
308308
)
309309
})
310310

311+
describe('while condition reference interpolation', () => {
312+
async function evaluateWhileConditionFor(condition: string, resolved: unknown) {
313+
const resolver = { resolveSingleReference: vi.fn().mockResolvedValue(resolved) }
314+
const orchestrator = new LoopOrchestrator(
315+
{ loopConfigs: new Map(), parallelConfigs: new Map(), nodes: new Map() } as any,
316+
createState(),
317+
resolver as any
318+
)
319+
const ctx = createContext({
320+
iteration: 0,
321+
currentIterationOutputs: new Map(),
322+
allIterationOutputs: [],
323+
loopType: 'while',
324+
condition,
325+
})
326+
327+
await orchestrator.evaluateInitialCondition(ctx, 'loop-1')
328+
329+
return mockExecuteInIsolatedVM.mock.calls[0][0].code as string
330+
}
331+
332+
it('does not let a quote in a resolved value escape into expression position', async () => {
333+
const payload = 'z" ) && Boolean( "INJ".length === 3 ) && Boolean( "never'
334+
335+
const code = await evaluateWhileConditionFor('<start.input> === "never"', payload)
336+
337+
expect(new Function(code)()).toBe(false)
338+
expect(code).toBe(`return Boolean(${JSON.stringify(payload)} === "never")`)
339+
})
340+
341+
it('escapes backslashes and newlines so the condition stays compilable', async () => {
342+
const code = await evaluateWhileConditionFor('<start.input> === "x"', 'a\\"\nb')
343+
344+
expect(() => new Function(code)).not.toThrow()
345+
expect(new Function(code)()).toBe(false)
346+
})
347+
348+
it('escapes line separators that are legal in JSON but not in every JS host', async () => {
349+
const code = await evaluateWhileConditionFor('<start.input> === "x"', 'a\u2028b\u2029c')
350+
351+
expect(code).not.toMatch(/[\u2028\u2029]/)
352+
expect(new Function(code)()).toBe(false)
353+
})
354+
355+
it.each([
356+
['TRUE', 'return Boolean(true)'],
357+
[' false ', 'return Boolean(false)'],
358+
[7, 'return Boolean(7)'],
359+
[true, 'return Boolean(true)'],
360+
[null, 'return Boolean(null)'],
361+
[{ a: 1 }, 'return Boolean({"a":1})'],
362+
])('serializes %o as a literal operand', async (resolved, expected) => {
363+
expect(await evaluateWhileConditionFor('<start.input>', resolved)).toBe(expected)
364+
})
365+
})
366+
367+
it('exits condition-driven loops at the hard iteration ceiling', async () => {
368+
const { orchestrator } = createOrchestrator()
369+
const ctx = createContext({
370+
iteration: 9999,
371+
loopType: 'while',
372+
condition: 'true',
373+
currentIterationOutputs: new Map(),
374+
allIterationOutputs: [],
375+
})
376+
377+
const result = await orchestrator.evaluateLoopContinuation(ctx, 'loop-1')
378+
379+
expect(result).toMatchObject({
380+
shouldContinue: false,
381+
shouldExit: true,
382+
selectedRoute: EDGE.LOOP_EXIT,
383+
})
384+
expect(mockExecuteInIsolatedVM).not.toHaveBeenCalled()
385+
})
386+
387+
it('keeps evaluating condition-driven loops below the hard iteration ceiling', async () => {
388+
const { orchestrator } = createOrchestrator()
389+
const ctx = createContext({
390+
iteration: 9997,
391+
loopType: 'while',
392+
condition: 'true',
393+
currentIterationOutputs: new Map(),
394+
allIterationOutputs: [],
395+
})
396+
397+
const result = await orchestrator.evaluateLoopContinuation(ctx, 'loop-1')
398+
399+
expect(result.shouldContinue).toBe(true)
400+
expect(mockExecuteInIsolatedVM).toHaveBeenCalled()
401+
})
402+
311403
it('exits doWhile loops when the configured iteration cap is reached', async () => {
312404
const { orchestrator } = createOrchestrator()
313405
const ctx = createContext({

apps/sim/executor/orchestrators/loop.ts

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,50 @@ const logger = createLogger('LoopOrchestrator')
3838

3939
const LOOP_CONDITION_TIMEOUT_MS = 5000
4040

41+
/**
42+
* Hard ceiling on iterations for condition-driven loops (`while`, and `doWhile`
43+
* with an explicit condition). These have no user-configured `maxIterations`, so
44+
* a condition that never goes false would otherwise spin an execution worker
45+
* indefinitely.
46+
*/
47+
const MAX_CONDITION_LOOP_ITERATIONS = 10000
48+
49+
/**
50+
* Serializes a resolved reference value into a JavaScript literal for the loop
51+
* condition expression.
52+
*
53+
* The result is concatenated into source that is compiled and run in the
54+
* execution isolate, so every value must be emitted as a self-contained literal.
55+
* Interpolating a string without escaping would let a `"` in the resolved data
56+
* terminate the literal and continue in expression position.
57+
*/
58+
function formatConditionOperand(resolved: unknown): string {
59+
if (typeof resolved === 'boolean' || typeof resolved === 'number') {
60+
return String(resolved)
61+
}
62+
63+
if (typeof resolved === 'string') {
64+
const lower = resolved.toLowerCase().trim()
65+
if (lower === 'true' || lower === 'false') {
66+
return lower
67+
}
68+
}
69+
70+
return toJsLiteral(resolved)
71+
}
72+
73+
/**
74+
* JSON-serializes a value and escapes the code points that are valid inside a
75+
* JSON string but historically hazardous inside a JavaScript source literal.
76+
*/
77+
function toJsLiteral(value: unknown): string {
78+
const serialized = JSON.stringify(value)
79+
if (serialized === undefined) {
80+
return 'undefined'
81+
}
82+
return serialized.replace(/\u2028/g, String.raw`\u2028`).replace(/\u2029/g, String.raw`\u2029`)
83+
}
84+
4185
async function replaceLoopConditionReferences(
4286
condition: string,
4387
replacer: (match: string) => Promise<string>
@@ -303,7 +347,22 @@ export class LoopOrchestrator {
303347
}
304348

305349
private hasReachedConfiguredIterationLimit(scope: LoopScope, nextIteration: number): boolean {
306-
if (scope.loopType !== 'doWhile' || scope.maxIterations === undefined) {
350+
if (scope.maxIterations === undefined) {
351+
if (
352+
(scope.loopType === 'while' || scope.loopType === 'doWhile') &&
353+
nextIteration >= MAX_CONDITION_LOOP_ITERATIONS
354+
) {
355+
logger.error('Condition loop hit the iteration ceiling, forcing exit', {
356+
loopType: scope.loopType,
357+
condition: scope.condition,
358+
maxConditionIterations: MAX_CONDITION_LOOP_ITERATIONS,
359+
})
360+
return true
361+
}
362+
return false
363+
}
364+
365+
if (scope.loopType !== 'doWhile') {
307366
return false
308367
}
309368
return nextIteration >= scope.maxIterations
@@ -724,17 +783,7 @@ export class LoopOrchestrator {
724783
resolvedType: resolved === null ? 'null' : typeof resolved,
725784
})
726785
if (resolved !== undefined) {
727-
if (typeof resolved === 'boolean' || typeof resolved === 'number') {
728-
return String(resolved)
729-
}
730-
if (typeof resolved === 'string') {
731-
const lower = resolved.toLowerCase().trim()
732-
if (lower === 'true' || lower === 'false') {
733-
return lower
734-
}
735-
return `"${resolved}"`
736-
}
737-
return JSON.stringify(resolved)
786+
return formatConditionOperand(resolved)
738787
}
739788
return match
740789
})

0 commit comments

Comments
 (0)