From 3f10365d62a9d2141ad55a9f72a18029189b6f6a Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 13:20:00 -0700 Subject: [PATCH 01/13] fix(executor): stop resolved data from breaking out of generated code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Condition expression is compiled by inlining each resolved reference as source text, and the literal that gets emitted only ever anticipated the quoting it chose itself. The author's quoting decides the real context, so `"".includes('urgent')` — a shape that works correctly with benign data — let webhook, chat, or form data close the author's string and run as JavaScript in the condition sandbox, which receives the workspace's whole decrypted environment as the `environmentVariables` global. Template literals, regex literals, and a crafted object key reached the same place. Both condition formatters now escape every terminator of every JavaScript string context rather than the one they open. `\"`, `` \` ``, `\$` and `\/` are identity escapes, so a condition compares exactly what it compared before; only its ability to parse as anything but data changes. A quoted object reference additionally escapes its JSON, the only reading of that shape that was not already a syntax error. Function blocks bind block outputs as context variables but inlined the remaining resolved values as literals, so a workflow variable a Variables block had assigned from trigger data, or a loop item, could close the string it landed in. Those bind now too. Numbers, booleans, null, and strings that name an environment variable stay inline: the first three cannot terminate a literal, and the last has to stay in source because the placeholder — never the secret — is what is inlined, and the execution-boundary compiler binds it downstream. Condition evaluation also stops shipping the full secret map to the sandbox: it mounts only the names its script references, so a future defect in this path reaches nothing the condition did not already name. Co-Authored-By: Claude Opus 5 (1M context) --- .../condition/condition-handler.test.ts | 44 +++++++ .../handlers/condition/condition-handler.ts | 32 +++++ apps/sim/executor/utils/code-formatting.ts | 33 +++++ apps/sim/executor/variables/resolver.test.ts | 120 +++++++++++++++++- apps/sim/executor/variables/resolver.ts | 101 +++++++++------ .../variables/resolvers/block.test.ts | 11 ++ .../sim/executor/variables/resolvers/block.ts | 9 +- 7 files changed, 304 insertions(+), 46 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index d12270cc45c..ef3614bb18c 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -202,6 +202,50 @@ describe('ConditionBlockHandler', () => { ) }) + it('mounts only the secrets the condition names', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + + const conditions = [ + { id: 'cond1', title: 'if', value: '"{{ROUTE_KEY}}" === "beta"' }, + { id: 'else1', title: 'else', value: '' }, + ] + + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.secretScope).toBe('selected') + expect(toolParams.mountedSecrets).toEqual(['ROUTE_KEY']) + }) + + it('denies every secret to a condition that names none', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + + const conditions = [ + { id: 'cond1', title: 'if', value: 'context.value > 5' }, + { id: 'else1', title: 'else', value: '' }, + ] + + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.secretScope).toBe('selected') + expect(toolParams.mountedSecrets).toEqual([]) + }) + + it('keeps the whole environment for a condition that reads the environment directly', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + + const conditions = [ + { id: 'cond1', title: 'if', value: 'environmentVariables.ROUTE_KEY === "beta"' }, + { id: 'else1', title: 'else', value: '' }, + ] + + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.secretScope).toBe('all') + }) + it('should never forward collected block outputs in the request body', async () => { mockCollectBlockData.mockReturnValueOnce({ blockData: { 'huge-block': { payload: 'x'.repeat(1024) } }, diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index d646428bd0f..aa817bec4cd 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -10,6 +10,7 @@ import type { BlockOutput } from '@/blocks/types' import { BlockType, DEFAULTS, EDGE } from '@/executor/constants' import type { BlockHandler, ExecutionContext } from '@/executor/types' import { collectBlockData } from '@/executor/utils/block-data' +import { createEnvVarPattern } from '@/executor/utils/reference-validation' import { buildBranchNodeId, extractBaseBlockId, @@ -88,6 +89,34 @@ function buildConditionScript(expressions: string[], evalContext: Record() + for (const match of code.matchAll(createEnvVarPattern())) { + named.add(String(match[1]).trim()) + } + return { secretScope: 'selected', mountedSecrets: [...named] } +} + /** * Runs condition code through the shared function execution boundary. * @@ -103,12 +132,15 @@ async function runConditionCode( currentNodeId?: string ): Promise { const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) + const { secretScope, mountedSecrets } = scopeConditionSecrets(code) return executeTool( 'function_execute', { code, timeout: CONDITION_TIMEOUT_MS, + secretScope, + mountedSecrets, envVars: normalizeStringRecord(ctx.environmentVariables), workflowVariables: normalizeWorkflowVariables(ctx.workflowVariables), blockData: {}, diff --git a/apps/sim/executor/utils/code-formatting.ts b/apps/sim/executor/utils/code-formatting.ts index a4a73dee8ab..f7db26e28fd 100644 --- a/apps/sim/executor/utils/code-formatting.ts +++ b/apps/sim/executor/utils/code-formatting.ts @@ -46,3 +46,36 @@ export function formatLiteralForCode(value: unknown, language: 'javascript' | 'p } return JSON.stringify(value) } + +/** + * Escapes text so it cannot terminate whatever JavaScript literal it is spliced into. + * + * Condition expressions are user-authored JavaScript into which resolved references are + * inlined as source, so the author's quoting — not this module's — decides which string + * context the value lands in. Escaping only the quote the emitted literal opens leaves + * `"`, a backtick, `${`, and `/` live, and `"".includes('urgent')` then lets + * trigger data close the author's string and run as code in the condition sandbox. + * + * So every terminator of every JavaScript string context is escaped, not just the one the + * emitted literal opens. `\"`, `` \` ``, `\$`, and `\/` are identity escapes in JavaScript, + * so the value a condition compares is byte-identical to what it was before — this is a + * syntax guard, not a value transform. + * + * JavaScript only: `\$`, `` \` `` and `\/` are not identity escapes in Python, where they + * would change the value. Code blocks bind their values as runtime context variables + * instead of splicing them, which is why they need no escaping in any language. + */ +export function escapeInertStringContent(value: string): string { + return value + .replace(/\\/g, '\\\\') + .replace(/['"`$/]/g, '\\$&') + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r') + .replace(/\u2028/g, '\\u2028') + .replace(/\u2029/g, '\\u2029') +} + +/** Wraps {@link escapeInertStringContent} output as a complete JavaScript string literal. */ +export function formatInertStringLiteral(value: string, quote: '"' | "'" = "'"): string { + return `${quote}${escapeInertStringContent(value)}${quote}` +} diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index aa0c8c93b42..722ef9948dc 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -144,6 +144,43 @@ async function resolveConditionExpression( return (result.conditions as Array<{ value: string }>)[0].value } +/** Resolves one condition expression against a producer output an attacker supplied. */ +async function resolveConditionWithBlockOutput(value: string, result: unknown): Promise { + const { ctx, resolver, state } = createResolver() + state.setBlockOutput('producer', { result } as never) + const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION) + const resolved = await resolver.resolveInputs( + ctx, + conditionBlock.id, + { conditions: JSON.stringify([{ id: 'condition-1', title: 'if', value }]) }, + conditionBlock + ) + return (resolved.conditions as Array<{ value: string }>)[0].value +} + +const INJECTION_CANARY = '__conditionInjectionCanary' + +/** + * Evaluates a resolved expression inside the same `Boolean(...)` wrapper the handler builds, + * reporting both the branch verdict and whether anything the resolved data carried executed. + */ +function runResolvedCondition(expression: string): { matched: boolean; injected: boolean } { + Reflect.set(globalThis, INJECTION_CANARY, 'not-executed') + try { + const matched = Boolean( + new Function(`const context = {};\nreturn Boolean(\n${expression}\n)`)() + ) + return { matched, injected: Reflect.get(globalThis, INJECTION_CANARY) !== 'not-executed' } + } catch { + return { + matched: false, + injected: Reflect.get(globalThis, INJECTION_CANARY) !== 'not-executed', + } + } finally { + Reflect.deleteProperty(globalThis, INJECTION_CANARY) + } +} + /** * Completes the round trip a condition actually takes: resolver, then the execution-boundary * compiler, then evaluation of the same `Boolean(...)` wrapper `condition-handler.ts` builds. @@ -255,6 +292,60 @@ describe('VariableResolver function block inputs', () => { ).resolves.toBe(true) }) + it('stops trigger data from breaking out of a quoted condition reference', async () => { + // Every quoting an author can put around a reference. The author picks the context; + // the resolved value must be data in all of them, not just the one it wraps itself in. + const quotings = [ + `"".includes('urgent')`, + '"" === "admin"', + '``.length > 0', + '//.test("x")', + ` === 'admin'`, + ] + const payloads = [ + `" + (globalThis.${INJECTION_CANARY} = "ran") + "`, + `\${(globalThis.${INJECTION_CANARY} = "ran")}`, + `' + (globalThis.${INJECTION_CANARY} = "ran") + '`, + `/ + (globalThis.${INJECTION_CANARY} = "ran") + /`, + ] + + for (const value of quotings) { + for (const payload of payloads) { + const expression = await resolveConditionWithBlockOutput(value, payload) + expect( + runResolvedCondition(expression).injected, + `condition ${value} executed trigger data: ${expression}` + ).toBe(false) + } + } + }) + + it('stops a trigger-supplied object from closing the string it is quoted inside', async () => { + // JSON's own structural quotes close the author's string, and the key is the attacker's. + const expression = await resolveConditionWithBlockOutput('"" === "{}"', { + [`+(globalThis.${INJECTION_CANARY}=1)+`]: 1, + }) + expect(runResolvedCondition(expression).injected).toBe(false) + }) + + it('keeps quoted and bare condition references comparing what they compared before', async () => { + const cases: Array<{ value: string; result: unknown; expected: boolean }> = [ + { value: ` === 'urgent'`, result: 'urgent', expected: true }, + { value: ` === 'urgent'`, result: 'other', expected: false }, + { value: `"".includes('urgent')`, result: 'urgent ticket', expected: true }, + { value: `"".includes('urgent')`, result: 'calm ticket', expected: false }, + { value: '``.length > 3', result: 'hello', expected: true }, + { value: ` === 'a"b'`, result: 'a"b', expected: true }, + { value: ' === `a$b/c`', result: 'a$b/c', expected: true }, + { value: `.count === 2`, result: { count: 2 }, expected: true }, + ] + + for (const { value, result, expected } of cases) { + const expression = await resolveConditionWithBlockOutput(value, result) + expect(runResolvedCondition(expression).matched, `condition ${value}`).toBe(expected) + } + }) + it('compares a bare string placeholder instead of throwing a reference error', async () => { await expect( evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'alice' }) @@ -719,6 +810,28 @@ describe('VariableResolver function block inputs', () => { expect(result.contextVariables).toEqual({ __blockRef_0: ref }) }) + it('binds a workflow variable carrying quote characters instead of splicing it into code', async () => { + // A Variables block can assign trigger data at runtime, so a variable's value is not + // necessarily the author's. Inlined as a literal it closed the string it landed in. + const { block, ctx, resolver } = createResolver('javascript') + const payload = `' + (globalThis.__functionInjection = 1) + '` + ctx.workflowVariables = { + 'var-1': { id: 'var-1', name: 'userinput', type: 'string', value: payload }, + } + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { code: `const x = ''; return x` }, + block + ) + + expect(result.resolvedInputs.code).toBe( + `const x = '' + JSON.stringify(globalThis["__blockRef_0"]) + ''; return x` + ) + expect(result.contextVariables).toEqual({ __blockRef_0: payload }) + }) + it('rewrites whole manifest workflow variables to lazy JavaScript array reads', async () => { const { block, ctx, resolver } = createResolver('javascript') const manifest = createTestManifest() @@ -791,8 +904,11 @@ describe('VariableResolver function block inputs', () => { ['0', 'key'], expect.objectContaining({ allowLargeValueRefs: true }) ) - expect(result.resolvedInputs.code).toBe('return "SIM-0"') - expect(result.contextVariables).toEqual({}) + // The navigated element binds like any other resolved value; what must not appear is + // the manifest, or the array it stands for. + expect(result.resolvedInputs.code).toBe('return globalThis["__blockRef_0"]') + expect(result.displayInputs.code).toBe('return "SIM-0"') + expect(result.contextVariables).toEqual({ __blockRef_0: 'SIM-0' }) }) it('resolves named loop result bracket paths in function code', async () => { diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 94c0bee918e..24f7e848059 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -18,6 +18,10 @@ import { isLikelyReferenceSegment } from '@/lib/workflows/sanitization/reference import { BlockType, parseReferencePath, REFERENCE } from '@/executor/constants' import type { ExecutionState, LoopScope } from '@/executor/execution/state' import type { ExecutionContext, UserFile } from '@/executor/types' +import { + escapeInertStringContent, + formatInertStringLiteral, +} from '@/executor/utils/code-formatting' import { createEnvVarPattern, createReferencePattern } from '@/executor/utils/reference-validation' import { BlockResolver } from '@/executor/variables/resolvers/block' import { EnvResolver } from '@/executor/variables/resolvers/env' @@ -621,34 +625,31 @@ export class VariableResolver { throw getNestedLargeValueMaterializationError() } - if ( - this.isWorkflowVariableReference(match) && - this.shouldUseContextVariable(effectiveValue) - ) { - const varName = `__blockRef_${Object.keys(contextVarAccumulator).length}` - contextVarAccumulator[varName] = effectiveValue - const replacement = this.formatContextVariableReference( - varName, - language, - template, - index, - effectiveValue - ) - displayResult += this.formatDisplayValueForCodeContext( + if (this.canInlineResolvedCodeLiteral(effectiveValue)) { + const replacement = this.blockResolver.formatValueForBlock( effectiveValue, - language, - template, - index + BlockType.FUNCTION, + language ) + displayResult += replacement return replacement } - const replacement = this.blockResolver.formatValueForBlock( + const varName = `__blockRef_${Object.keys(contextVarAccumulator).length}` + contextVarAccumulator[varName] = effectiveValue + const replacement = this.formatContextVariableReference( + varName, + language, + template, + index, + effectiveValue + ) + displayResult += this.formatDisplayValueForCodeContext( effectiveValue, - BlockType.FUNCTION, - language + language, + template, + index ) - displayResult += replacement return replacement } catch (error) { replacementError = error instanceof Error ? error : new Error(String(error)) @@ -898,13 +899,30 @@ export class VariableResolver { }) } - private isWorkflowVariableReference(reference: string): boolean { - const parts = parseReferencePath(reference) - return parts[0] === REFERENCE.PREFIX.VARIABLE - } - - private shouldUseContextVariable(value: unknown): boolean { - return typeof value === 'object' && value !== null + /** + * Whether a resolved value may stay a literal in generated code rather than being bound + * as a context variable. + * + * Splicing a value into user-authored code hands the author's quoting the decision of how + * that value parses, so only values that cannot terminate a literal may stay inline. + * Numbers, booleans, and null render as digits or keywords. A string qualifies only when + * it names an environment variable and carries no character that could close a string in + * any supported language: that shape has to stay in source because the placeholder, never + * the secret, is what gets inlined, and the execution-boundary compiler binds it + * downstream — that is how `` reaches its value. + * + * Everything else binds, which is what block outputs have always done. Inlining the rest + * is what let a runtime-assigned variable or loop item carrying trigger data close the + * string it landed in and run as code. + */ + private canInlineResolvedCodeLiteral(value: unknown): boolean { + if (value === null || typeof value === 'number' || typeof value === 'boolean') { + return true + } + if (typeof value !== 'string') { + return false + } + return createEnvVarPattern().test(value) && !/['"`$\\\n\r\u2028\u2029]/.test(value) } private formatJavaScriptAsyncExpression( @@ -1532,19 +1550,12 @@ export class VariableResolver { } if (typeof resolved === 'string') { - const escaped = resolved - .replace(/\\/g, '\\\\') - .replace(/'/g, "\\'") - .replace(/\n/g, '\\n') - .replace(/\r/g, '\\r') - .replace(/\u2028/g, '\\u2028') - .replace(/\u2029/g, '\\u2029') - const formatted = `'${escaped}'` + const formatted = formatInertStringLiteral(resolved) projectedReferenceResult += containsResolvedSecret ? match : formatted return formatted } if (typeof resolved === 'object' && resolved !== null) { - const formatted = JSON.stringify(resolved) + const formatted = this.formatConditionJson(resolved, template, index) projectedReferenceResult += containsResolvedSecret ? match : formatted return formatted } @@ -1576,6 +1587,22 @@ export class VariableResolver { return result } + /** + * Renders a resolved object for a Condition expression. + * + * In expression position the author is comparing or navigating the object itself, so the + * JSON has to stay a literal. Inside a quoted string the same text is data, and its own + * structural quotes would close the author's string — `"" === "{}"` with a + * crafted key emits `"{"+attackerCode()+":1}"`, which parses as concatenation and runs. + * Escaping the text there keeps it inside the string the author opened, which is also the + * only reading of a quoted object reference that is not a syntax error. + */ + private formatConditionJson(value: object, template: string, matchIndex: number): string { + const json = JSON.stringify(value) + const quoteContext = this.getCodeStringQuoteContext(template, matchIndex, 'javascript') + return quoteContext === null ? json : escapeInertStringContent(json) + } + private async resolveReference(reference: string, context: ResolutionContext): Promise { for (const resolver of this.resolvers) { if (resolver.canResolve(reference)) { diff --git a/apps/sim/executor/variables/resolvers/block.test.ts b/apps/sim/executor/variables/resolvers/block.test.ts index 7fa1576742b..de4cb9da017 100644 --- a/apps/sim/executor/variables/resolvers/block.test.ts +++ b/apps/sim/executor/variables/resolvers/block.test.ts @@ -711,6 +711,17 @@ describe('BlockResolver', () => { expect(resolver.formatValueForBlock('tab\there', 'condition')).toBe('"tab\there"') }) + it.concurrent('should escape the quotes it does not open for condition block', () => { + // The author's quoting decides which literal this lands in, so escaping only the + // double quote this wrapper opens leaves the other contexts breakable. + const resolver = new BlockResolver(createTestWorkflow()) + expect(resolver.formatValueForBlock("' + evil() + '", 'condition')).toBe( + '"\\\' + evil() + \\\'"' + ) + expect(resolver.formatValueForBlock(`\${evil()}`, 'condition')).toBe(`"\\\${evil()}"`) + expect(resolver.formatValueForBlock('`evil()`', 'condition')).toBe('"\\`evil()\\`"') + }) + it.concurrent('should format object for condition block', () => { const resolver = new BlockResolver(createTestWorkflow()) const result = resolver.formatValueForBlock({ key: 'value' }, 'condition') diff --git a/apps/sim/executor/variables/resolvers/block.ts b/apps/sim/executor/variables/resolvers/block.ts index 83f20eecd27..2c3f634e08b 100644 --- a/apps/sim/executor/variables/resolvers/block.ts +++ b/apps/sim/executor/variables/resolvers/block.ts @@ -13,7 +13,7 @@ import { resolveBlockReference, resolveBlockReferenceAsync, } from '@/executor/utils/block-reference' -import { formatLiteralForCode } from '@/executor/utils/code-formatting' +import { formatInertStringLiteral, formatLiteralForCode } from '@/executor/utils/code-formatting' import { buildClonedSubflowId, extractOuterBranchIndex } from '@/executor/utils/subflow-utils' import { type AsyncPathNavigator, @@ -414,12 +414,7 @@ export class BlockResolver implements Resolver { private stringifyForCondition(value: any): string { if (typeof value === 'string') { - const sanitized = value - .replace(/\\/g, '\\\\') - .replace(/"/g, '\\"') - .replace(/\n/g, '\\n') - .replace(/\r/g, '\\r') - return `"${sanitized}"` + return formatInertStringLiteral(value, '"') } if (value === null) { return 'null' From bf0a80c1abf16252717f04c4270bc96e6340a761 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 13:36:13 -0700 Subject: [PATCH 02/13] fix(executor): parse condition objects instead of splicing their JSON Escaping a quoted object's JSON left the quote scanner load-bearing for injection: it does not track regex literals, so a quote inside one desynchronizes it and a later object reference is reported as unquoted, which put raw attacker-shaped JSON back into source. A reference inside a regex literal reached the same place through its unescaped slashes. Objects outside a string are now parsed at runtime from a fully escaped literal. The value is identical to the object literal it replaces, and the emitted form carries no quote, slash, backtick or `${`, so it stays inert whichever context the scanner reports. Scoping now reads the expressions for a direct `environmentVariables` access rather than the whole generated script, so a source block's output containing that word can no longer widen the mounted secret set. The placeholder scan still reads the built script, which is the text the execution-boundary compiler substitutes over. Co-Authored-By: Claude Opus 5 (1M context) --- .../condition/condition-handler.test.ts | 21 +++++++++++ .../handlers/condition/condition-handler.ts | 31 +++++++++------- apps/sim/executor/variables/resolver.test.ts | 35 +++++++++++++++++++ apps/sim/executor/variables/resolver.ts | 21 ++++++----- 4 files changed, 88 insertions(+), 20 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index ef3614bb18c..42fc7a3bc75 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -232,6 +232,27 @@ describe('ConditionBlockHandler', () => { expect(toolParams.mountedSecrets).toEqual([]) }) + it('does not let resolved data widen the mounted secrets by naming the environment', async () => { + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + mockContext.blockStates.set('source-block-1', { + output: { text: 'environmentVariables.OPENAI_API_KEY' }, + executed: true, + executionTime: 0, + } as BlockState) + + const conditions = [ + { id: 'cond1', title: 'if', value: `context.text === 'x'` }, + { id: 'else1', title: 'else', value: '' }, + ] + + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.code).toContain('environmentVariables.OPENAI_API_KEY') + expect(toolParams.secretScope).toBe('selected') + expect(toolParams.mountedSecrets).toEqual([]) + }) + it('keeps the whole environment for a condition that reads the environment directly', async () => { mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index aa817bec4cd..29833488301 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -92,21 +92,26 @@ function buildConditionScript(expressions: string[], evalContext: Record /\benvironmentVariables\s*[.[]/.test(expression))) { return { secretScope: 'all', mountedSecrets: [] } } @@ -129,10 +134,11 @@ function scopeConditionSecrets(code: string): { async function runConditionCode( ctx: ExecutionContext, code: string, + expressions: string[], currentNodeId?: string ): Promise { const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) - const { secretScope, mountedSecrets } = scopeConditionSecrets(code) + const { secretScope, mountedSecrets } = scopeConditionSecrets(code, expressions) return executeTool( 'function_execute', @@ -182,6 +188,7 @@ async function evaluateConditionList( const result = await runConditionCode( ctx, buildConditionScript(expressions, evalContext), + expressions, currentNodeId ) @@ -266,7 +273,7 @@ async function evaluateSingleCondition( currentNodeId?: string ): Promise { const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}` - const result = await runConditionCode(ctx, code, currentNodeId) + const result = await runConditionCode(ctx, code, [expression], currentNodeId) if (!result.success) { if (result.retryable === false) { diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 722ef9948dc..0db6ce7d7cf 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -328,6 +328,41 @@ describe('VariableResolver function block inputs', () => { expect(runResolvedCondition(expression).injected).toBe(false) }) + it('stops a trigger-supplied object from escaping wherever the quote scanner mis-reads', async () => { + // A regex literal is not tracked by the quote scanner, and a quote inside one + // desynchronizes it for everything that follows, so the emitted object must be inert + // whichever context the scanner reports. + const payloads = [ + { [`+(globalThis.${INJECTION_CANARY}=1)+`]: 1 }, + { forged: `/ + (globalThis.${INJECTION_CANARY}=1) + /` }, + { closed: `" + (globalThis.${INJECTION_CANARY}=1) + "` }, + ] + const quotings = [ + '//.test("x")', + `/['"]/.test('a') && .count === 2`, + `/['"]/.test('a') && "" === "{}"`, + '.count === 2', + ] + + for (const value of quotings) { + for (const payload of payloads) { + const expression = await resolveConditionWithBlockOutput(value, payload) + expect( + runResolvedCondition(expression).injected, + `condition ${value} executed object data: ${expression}` + ).toBe(false) + } + } + }) + + it('keeps navigating an object reference the scanner reports as unquoted', async () => { + const expression = await resolveConditionWithBlockOutput('.count === 2', { + count: 2, + note: `a "quoted" / slashed ' value`, + }) + expect(runResolvedCondition(expression).matched).toBe(true) + }) + it('keeps quoted and bare condition references comparing what they compared before', async () => { const cases: Array<{ value: string; result: unknown; expected: boolean }> = [ { value: ` === 'urgent'`, result: 'urgent', expected: true }, diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 24f7e848059..214a49348b3 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -1590,17 +1590,22 @@ export class VariableResolver { /** * Renders a resolved object for a Condition expression. * - * In expression position the author is comparing or navigating the object itself, so the - * JSON has to stay a literal. Inside a quoted string the same text is data, and its own - * structural quotes would close the author's string — `"" === "{}"` with a - * crafted key emits `"{"+attackerCode()+":1}"`, which parses as concatenation and runs. - * Escaping the text there keeps it inside the string the author opened, which is also the - * only reading of a quoted object reference that is not a syntax error. + * Inside a quoted string the object is data, so its JSON is escaped to stay inside the + * string the author opened: raw, the JSON's own structural quotes close that string, and + * `"" === "{}"` with a crafted key emits `"{"+attackerCode()+":1}"`, which + * parses as concatenation and runs. + * + * Everywhere else the object is parsed at runtime rather than spliced as source. The value + * is identical to the object literal it replaces, but the payload is a fully escaped + * literal, so a crafted key can neither close a string nor forge a regex delimiter — which + * matters because the emitted form must be safe even when the quote scanner reads the + * surrounding context wrongly, and a quote inside a regex literal is enough to do that. + * Splicing raw JSON would make that heuristic load-bearing for injection. */ private formatConditionJson(value: object, template: string, matchIndex: number): string { - const json = JSON.stringify(value) + const escaped = escapeInertStringContent(JSON.stringify(value)) const quoteContext = this.getCodeStringQuoteContext(template, matchIndex, 'javascript') - return quoteContext === null ? json : escapeInertStringContent(json) + return quoteContext === null ? `JSON.parse('${escaped}')` : escaped } private async resolveReference(reference: string, context: ResolutionContext): Promise { From 73bba519e87244ed11c1c62d1aa317ca827d9fd5 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 13:38:50 -0700 Subject: [PATCH 03/13] fix(executor): widen the condition environment read to any mention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A member-access pattern decides whether a condition keeps the full secret map, and every shape it fails to anticipate — `environmentVariables?.FLAG`, a read through `Object.keys` — silently narrows what that expression can see and routes the run down a branch the author did not write. Matching the bare identifier inside the expressions costs only the narrowing, and never mounts more than this path mounted before it existed. Co-Authored-By: Claude Opus 5 (1M context) --- .../condition/condition-handler.test.ts | 27 +++++++++++++------ .../handlers/condition/condition-handler.ts | 14 +++++++--- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index 42fc7a3bc75..5fabaa863a6 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -254,17 +254,28 @@ describe('ConditionBlockHandler', () => { }) it('keeps the whole environment for a condition that reads the environment directly', async () => { - mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) - - const conditions = [ - { id: 'cond1', title: 'if', value: 'environmentVariables.ROUTE_KEY === "beta"' }, - { id: 'else1', title: 'else', value: '' }, + // Every shape an expression can reach the map through, including the ones a member-access + // pattern would miss — narrowing one of those would route the run silently. + const reads = [ + 'environmentVariables.ROUTE_KEY === "beta"', + 'environmentVariables["ROUTE_KEY"] === "beta"', + 'environmentVariables?.ROUTE_KEY === "beta"', + 'Object.keys(environmentVariables).length > 0', ] - await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + for (const value of reads) { + mockExecuteTool.mockReset() + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + const conditions = [ + { id: 'cond1', title: 'if', value }, + { id: 'else1', title: 'else', value: '' }, + ] - const [, toolParams] = mockExecuteTool.mock.calls[0] - expect(toolParams.secretScope).toBe('all') + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.secretScope, `condition ${value}`).toBe('all') + } }) it('should never forward collected block outputs in the request body', async () => { diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index 29833488301..6459d7940dc 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -100,9 +100,15 @@ function buildConditionScript(expressions: string[], evalContext: Record /\benvironmentVariables\s*[.[]/.test(expression))) { + if (expressions.some((expression) => /\benvironmentVariables\b/.test(expression))) { return { secretScope: 'all', mountedSecrets: [] } } From c28a7e4bc3c9c385a2f884e1ed292044ba997949 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 13:46:37 -0700 Subject: [PATCH 04/13] fix(executor): teach the code scanner about regex literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A regex body is the one place a lone quote is not a string delimiter, and the scanner did not track regex literals at all: `/['"]/` left it believing everything after it sat inside a string. Every later reference was then formatted for a context it was not in — a quoted object reference stayed raw source, and after the previous commit a bare one was emitted as escaped JSON, which cannot parse, so a valid condition threw instead of routing. The scan now enters regex mode where a `/` can only be a regex — division always follows a value, so the preceding token decides — and tracks escapes and character classes until the closing delimiter. A reference inside a regex reports its own context, so its JSON is escaped as pattern text rather than spliced with delimiters the data could forge. The same scanner decides how function-block references are spliced, so this also repairs quoting for code that matches on a quote-bearing pattern. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 40 +++++++ apps/sim/executor/variables/resolver.ts | 120 ++++++++++++++++++- 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 0db6ce7d7cf..f46637bb734 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -363,6 +363,46 @@ describe('VariableResolver function block inputs', () => { expect(runResolvedCondition(expression).matched).toBe(true) }) + it('evaluates references that follow a regex literal, quote-bearing or not', async () => { + // A regex body is the one place a lone quote is not a string delimiter. Reading it as one + // left every later reference formatted for a context it was not in — a quoted object + // reference stayed raw source, and a bare one was emitted as escaped JSON that cannot parse. + const cases: Array<{ value: string; result: unknown; expected: boolean }> = [ + // Every case reaches its reference — a short-circuit would pass on a formatter that + // emits source the sandbox cannot parse, which is the failure being pinned here. + { + value: `/['"a]/.test('a') && .count === 2`, + result: { count: 2 }, + expected: true, + }, + { value: `/['"]/.test('a') || === 'x'`, result: 'x', expected: true }, + { + value: `/it's/.test('a') || "".includes('b')`, + result: 'abc', + expected: true, + }, + { + value: `/[a-z]/.test('a') && .count === 2`, + result: { count: 2 }, + expected: true, + }, + // Division, not a regex: the scan must not swallow the rest of the expression. + { value: `.total / 2 === 5`, result: { total: 10 }, expected: true }, + { + value: `(.total / 2) === 5 && ''.length > 0`, + result: { total: 10 }, + expected: true, + }, + ] + + for (const { value, result, expected } of cases) { + const expression = await resolveConditionWithBlockOutput(value, result) + const verdict = runResolvedCondition(expression) + expect(verdict.injected, `condition ${value} executed data: ${expression}`).toBe(false) + expect(verdict.matched, `condition ${value} resolved to: ${expression}`).toBe(expected) + } + }) + it('keeps quoted and bare condition references comparing what they compared before', async () => { const cases: Array<{ value: string; result: unknown; expected: boolean }> = [ { value: ` === 'urgent'`, result: 'urgent', expected: true }, diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 214a49348b3..425af7d3ef7 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -146,7 +146,12 @@ function isStructurallyInertConditionLiteral(value: string): boolean { } type ShellQuoteContext = 'single' | 'double' | null -type CodeStringQuoteContext = ShellQuoteContext | 'triple-single' | 'triple-double' | 'template' +type CodeStringQuoteContext = + | ShellQuoteContext + | 'triple-single' + | 'triple-double' + | 'template' + | 'regex' type CodeScanMode = | { type: 'normal' } | { type: 'single' } @@ -157,6 +162,37 @@ type CodeScanMode = | { type: 'template-expression'; depth: number } | { type: 'line-comment' } | { type: 'block-comment' } + | { type: 'regex'; inCharacterClass: boolean } + +/** + * Characters after which a `/` opens a regular expression rather than dividing. + * + * The scanner has to tell the two apart, because a regex body is the one place a lone quote + * is not a string delimiter: `/['"]/` left the scan believing everything after it sat inside + * a string, and every reference past that point was then formatted for the wrong context. + * Division always follows a value — an identifier, literal, `)`, or `]` — so anything else + * ending the preceding token means a regex may start. + */ +const JAVASCRIPT_REGEX_ALLOWED_AFTER = new Set('(,=:[!&|?{};+-*%^~<>/'.split('')) + +const WHITESPACE_CHAR = /\s/ + +/** Keywords a regex may directly follow, where the preceding token is a word rather than punctuation. */ +const JAVASCRIPT_REGEX_ALLOWED_AFTER_KEYWORDS = new Set([ + 'return', + 'typeof', + 'instanceof', + 'in', + 'of', + 'new', + 'delete', + 'void', + 'case', + 'do', + 'else', + 'yield', + 'await', +]) export class VariableResolver { private resolvers: Resolver[] @@ -1073,6 +1109,35 @@ export class VariableResolver { ) } + /** + * Whether a `/` at this point opens a regular expression rather than dividing. + * + * Division always follows a value, so the preceding token decides: an identifier that is not + * one of the keywords a regex may follow, a number, a `)`, or a `]` means division, and + * anything else means a regex may start. Guessing wrong is not silent — the scan would swallow + * text up to the next `/` — so the check reads the actual preceding token rather than assuming. + */ + private canStartJavaScriptRegex(template: string, previousSignificantIndex: number): boolean { + if (previousSignificantIndex < 0) { + return true + } + const previous = template[previousSignificantIndex] + if (JAVASCRIPT_REGEX_ALLOWED_AFTER.has(previous)) { + return true + } + if (!this.isJavaScriptIdentifierChar(previous)) { + return false + } + + let start = previousSignificantIndex + while (start > 0 && this.isJavaScriptIdentifierChar(template[start - 1])) { + start-- + } + return JAVASCRIPT_REGEX_ALLOWED_AFTER_KEYWORDS.has( + template.slice(start, previousSignificantIndex + 1) + ) + } + private matchesKeywordAt(template: string, index: number, keyword: string): boolean { if (!template.startsWith(keyword, index)) { return false @@ -1200,6 +1265,7 @@ export class VariableResolver { ): CodeStringQuoteContext { const isPython = language === 'python' const modes: CodeScanMode[] = [{ type: 'normal' }] + let lastSignificantIndex = -1 for (let i = 0; i < index; i++) { const char = template[i] @@ -1221,6 +1287,31 @@ export class VariableResolver { continue } + if (mode.type === 'regex') { + if (char === '\\') { + i++ + continue + } + // A regex literal cannot span a line, so an unterminated one means the `/` was + // division after all; dropping the mode keeps the rest of the scan honest. + if (char === '\n') { + modes.pop() + continue + } + if (char === '[') { + mode.inCharacterClass = true + continue + } + if (char === ']') { + mode.inCharacterClass = false + continue + } + if (char === '/' && !mode.inCharacterClass) { + modes.pop() + } + continue + } + if (mode.type === 'single' || mode.type === 'double') { const quote = mode.type === 'single' ? "'" : '"' if (char === '\\') { @@ -1273,6 +1364,18 @@ export class VariableResolver { i++ continue } + const previousSignificantIndex = lastSignificantIndex + if (!WHITESPACE_CHAR.test(char)) { + lastSignificantIndex = i + } + if ( + !isPython && + char === '/' && + this.canStartJavaScriptRegex(template, previousSignificantIndex) + ) { + modes.push({ type: 'regex', inCharacterClass: false }) + continue + } if (isPython && char === "'" && next === "'" && template[i + 2] === "'") { modes.push({ type: 'triple-single' }) i += 2 @@ -1322,6 +1425,18 @@ export class VariableResolver { i++ continue } + const previousSignificantIndex = lastSignificantIndex + if (!WHITESPACE_CHAR.test(char)) { + lastSignificantIndex = i + } + if ( + !isPython && + char === '/' && + this.canStartJavaScriptRegex(template, previousSignificantIndex) + ) { + modes.push({ type: 'regex', inCharacterClass: false }) + continue + } if (isPython && char === "'" && next === "'" && template[i + 2] === "'") { modes.push({ type: 'triple-single' }) i += 2 @@ -1338,6 +1453,9 @@ export class VariableResolver { } const mode = modes[modes.length - 1] + if (mode.type === 'regex') { + return 'regex' + } if ( mode.type === 'single' || mode.type === 'double' || From 47da972c76fb958246da241630c912ad947398dd Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 13:59:14 -0700 Subject: [PATCH 05/13] fix(executor): keep resolved data out of the condition secret decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The built script carries the source block's output as data, so scanning it for placeholders let a caller choose which secret materializes beside its own payload: `{{SECRET}}` in trigger data mounted that secret and had the compiler expand it into the serialized context. Both scans now read the expressions, which is where every legitimate route to a secret runs — including a workflow variable holding `{{NAME}}`, since the resolver inlines that value into the expression before this handler sees it. `throw` joins the keywords a regex may follow. `throw /re/` is legal, and without it the scan reads the pattern body as code and mis-reports the context of everything after it. Co-Authored-By: Claude Opus 5 (1M context) --- .../condition/condition-handler.test.ts | 9 +++-- .../handlers/condition/condition-handler.ts | 37 ++++++++++--------- apps/sim/executor/variables/resolver.ts | 1 + 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index 5fabaa863a6..7b0f31c8dc7 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -232,10 +232,13 @@ describe('ConditionBlockHandler', () => { expect(toolParams.mountedSecrets).toEqual([]) }) - it('does not let resolved data widen the mounted secrets by naming the environment', async () => { + it('does not let resolved data decide which secrets the sandbox holds', async () => { + // The script carries the source block's output as data. Reading that data for either + // signal would let a caller pick what materializes beside it — the whole map by naming + // the global, or one secret by naming its placeholder. mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) mockContext.blockStates.set('source-block-1', { - output: { text: 'environmentVariables.OPENAI_API_KEY' }, + output: { text: 'environmentVariables.OPENAI_API_KEY {{OPENAI_API_KEY}}' }, executed: true, executionTime: 0, } as BlockState) @@ -248,7 +251,7 @@ describe('ConditionBlockHandler', () => { await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) const [, toolParams] = mockExecuteTool.mock.calls[0] - expect(toolParams.code).toContain('environmentVariables.OPENAI_API_KEY') + expect(toolParams.code).toContain('{{OPENAI_API_KEY}}') expect(toolParams.secretScope).toBe('selected') expect(toolParams.mountedSecrets).toEqual([]) }) diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index 6459d7940dc..626b5480961 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -97,23 +97,22 @@ function buildConditionScript(expressions: string[], evalContext: Record() - for (const match of code.matchAll(createEnvVarPattern())) { - named.add(String(match[1]).trim()) + for (const expression of expressions) { + for (const match of expression.matchAll(createEnvVarPattern())) { + named.add(String(match[1]).trim()) + } } return { secretScope: 'selected', mountedSecrets: [...named] } } @@ -144,7 +145,7 @@ async function runConditionCode( currentNodeId?: string ): Promise { const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) - const { secretScope, mountedSecrets } = scopeConditionSecrets(code, expressions) + const { secretScope, mountedSecrets } = scopeConditionSecrets(expressions) return executeTool( 'function_execute', diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 425af7d3ef7..2c48b53d9cb 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -188,6 +188,7 @@ const JAVASCRIPT_REGEX_ALLOWED_AFTER_KEYWORDS = new Set([ 'delete', 'void', 'case', + 'throw', 'do', 'else', 'yield', From 184460ad795aadd37a73005d5fbfeb45834a0116 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 14:09:54 -0700 Subject: [PATCH 06/13] fix(executor): read what a closing parenthesis closed `)` ends a value in `(a + b) / 2` and a control-flow head in `if (a) /re/.test(b)`, and the character alone does not say which. Treating it as one or the other unconditionally misreads the other: as a value, a statement-position regex is scanned as code, and a quote inside it decides how every reference sharing that line is spliced; as a head, ordinary division opens a regex that swallows the quotes after it. The scan now records which kind of parenthesis each `)` closed, by reading the keyword in front of its opener, and answers from that. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 28 +++++++++++ apps/sim/executor/variables/resolver.ts | 53 ++++++++++++++++++-- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index f46637bb734..fb068f5c9ac 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -885,6 +885,34 @@ describe('VariableResolver function block inputs', () => { expect(result.contextVariables).toEqual({ __blockRef_0: ref }) }) + it('reads the context of a reference that follows a statement-position regex', async () => { + // `)` ends a value in `(a + b) / 2` and a control-flow head in `if (a) /re/.test(b)`, and + // the closing parenthesis alone does not say which. Guessing either way misreads one of + // them, and a quote inside the regex then decides how every later reference is spliced. + const { block, ctx, resolver } = createResolver('javascript') + + // Both cases stay on one line: a string mode ends at a newline, so only a reference sharing + // the line with the misread slash sees the wrong context. + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { + code: [ + `if (params.a) /['"]/.test('')`, + `const divided = (params.c + 1) / 2 + Number('')`, + 'return divided', + ].join('\n'), + }, + block + ) + + const code = result.resolvedInputs.code as string + // Statement-position regex: the reference after it is inside the author's quotes. + expect(code).toContain(`.test('' + JSON.stringify(globalThis["__blockRef_0"]) + '')`) + // Division after a value: the slash must not open a regex that swallows the quotes. + expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) + }) + it('binds a workflow variable carrying quote characters instead of splicing it into code', async () => { // A Variables block can assign trigger data at runtime, so a variable's value is not // necessarily the author's. Inlined as a literal it closed the string it landed in. diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 2c48b53d9cb..fd86e881def 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -172,9 +172,17 @@ type CodeScanMode = * a string, and every reference past that point was then formatted for the wrong context. * Division always follows a value — an identifier, literal, `)`, or `]` — so anything else * ending the preceding token means a regex may start. + * + * `)` is the one that is not decidable from the character alone: it ends a value in + * `(a + b) / 2` and a control-flow head in `if (a) /re/.test(b)`. Reading it as either one + * unconditionally breaks the other, so the scan remembers which kind of parenthesis each `)` + * closed rather than guessing — see {@link CONTROL_FLOW_HEAD_KEYWORDS}. */ const JAVASCRIPT_REGEX_ALLOWED_AFTER = new Set('(,=:[!&|?{};+-*%^~<>/'.split('')) +/** Keywords whose parenthesized head is followed by a statement, where a regex may start. */ +const CONTROL_FLOW_HEAD_KEYWORDS = new Set(['if', 'while', 'for', 'switch', 'catch', 'with']) + const WHITESPACE_CHAR = /\s/ /** Keywords a regex may directly follow, where the preceding token is a word rather than punctuation. */ @@ -1118,11 +1126,18 @@ export class VariableResolver { * anything else means a regex may start. Guessing wrong is not silent — the scan would swallow * text up to the next `/` — so the check reads the actual preceding token rather than assuming. */ - private canStartJavaScriptRegex(template: string, previousSignificantIndex: number): boolean { + private canStartJavaScriptRegex( + template: string, + previousSignificantIndex: number, + controlHeadParenCloses: ReadonlySet + ): boolean { if (previousSignificantIndex < 0) { return true } const previous = template[previousSignificantIndex] + if (previous === ')') { + return controlHeadParenCloses.has(previousSignificantIndex) + } if (JAVASCRIPT_REGEX_ALLOWED_AFTER.has(previous)) { return true } @@ -1139,6 +1154,26 @@ export class VariableResolver { ) } + /** + * Whether the `(` at this index opens a control-flow head rather than a value. + * + * What follows the matching `)` differs entirely between the two — a statement, where a regex + * literal may begin, versus an operator, where a `/` divides — and the closing parenthesis + * carries no trace of which it was. Reading the keyword in front of the opening one is what + * lets `if (a) /re/.test(b)` and `(a + b) / 2` both scan correctly. + */ + private opensControlFlowHead(template: string, index: number): boolean { + let end = index + while (end > 0 && WHITESPACE_CHAR.test(template[end - 1])) { + end-- + } + let start = end + while (start > 0 && this.isJavaScriptIdentifierChar(template[start - 1])) { + start-- + } + return CONTROL_FLOW_HEAD_KEYWORDS.has(template.slice(start, end)) + } + private matchesKeywordAt(template: string, index: number, keyword: string): boolean { if (!template.startsWith(keyword, index)) { return false @@ -1267,6 +1302,8 @@ export class VariableResolver { const isPython = language === 'python' const modes: CodeScanMode[] = [{ type: 'normal' }] let lastSignificantIndex = -1 + const openParenIsControlHead: boolean[] = [] + const controlHeadParenCloses = new Set() for (let i = 0; i < index; i++) { const char = template[i] @@ -1369,10 +1406,15 @@ export class VariableResolver { if (!WHITESPACE_CHAR.test(char)) { lastSignificantIndex = i } + if (char === '(') { + openParenIsControlHead.push(this.opensControlFlowHead(template, i)) + } else if (char === ')') { + if (openParenIsControlHead.pop()) controlHeadParenCloses.add(i) + } if ( !isPython && char === '/' && - this.canStartJavaScriptRegex(template, previousSignificantIndex) + this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses) ) { modes.push({ type: 'regex', inCharacterClass: false }) continue @@ -1430,10 +1472,15 @@ export class VariableResolver { if (!WHITESPACE_CHAR.test(char)) { lastSignificantIndex = i } + if (char === '(') { + openParenIsControlHead.push(this.opensControlFlowHead(template, i)) + } else if (char === ')') { + if (openParenIsControlHead.pop()) controlHeadParenCloses.add(i) + } if ( !isPython && char === '/' && - this.canStartJavaScriptRegex(template, previousSignificantIndex) + this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses) ) { modes.push({ type: 'regex', inCharacterClass: false }) continue From d5c2b892ccbc10bc613ea2a3d7eb9a2e527acce3 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 14:14:37 -0700 Subject: [PATCH 07/13] fix(executor): decide the environment read from the author's own text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolved data is quoted inside the expression the handler sees, so a payload containing `environmentVariables` read exactly like the author reaching for the map, and a caller could restore the full secret set by sending the word. The resolver now records that answer per branch from the pre-resolution expression, where only the author's text exists, and the handler reads the record — falling back to scanning only when a caller did not resolve through the resolver, since narrowing a real read would route the run silently. A closed regex literal also counts as a value now, so the division in `/re/.source.length / 2` no longer opens a second regex that swallows the quotes after it. Co-Authored-By: Claude Opus 5 (1M context) --- .../condition/condition-handler.test.ts | 22 +++++++ .../handlers/condition/condition-handler.ts | 61 +++++++++++-------- apps/sim/executor/variables/resolver.test.ts | 38 +++++++++++- apps/sim/executor/variables/resolver.ts | 38 ++++++++++-- 4 files changed, 126 insertions(+), 33 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index 7b0f31c8dc7..f7db2aa7846 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -256,6 +256,28 @@ describe('ConditionBlockHandler', () => { expect(toolParams.mountedSecrets).toEqual([]) }) + it('trusts the resolver record over the word appearing in a resolved expression', async () => { + // The resolver saw the author's text before any value was inlined; the expression by now + // carries trigger data, where the same word means nothing. + mockExecuteTool.mockResolvedValueOnce(matchedAt(0)) + + const conditions = [ + { + id: 'cond1', + title: 'if', + value: `'environmentVariables.OPENAI_API_KEY' === 'x'`, + _readsEnvironmentVariables: false, + }, + { id: 'else1', title: 'else', value: '' }, + ] + + await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) }) + + const [, toolParams] = mockExecuteTool.mock.calls[0] + expect(toolParams.secretScope).toBe('selected') + expect(toolParams.mountedSecrets).toEqual([]) + }) + it('keeps the whole environment for a condition that reads the environment directly', async () => { // Every shape an expression can reach the map through, including the ones a member-access // pattern would miss — narrowing one of those would route the run silently. diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index 626b5480961..1db5a685e0e 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -17,6 +17,7 @@ import { extractBranchIndex, isBranchNodeId, } from '@/executor/utils/subflow-utils' +import { CONDITION_READS_ENVIRONMENT_KEY } from '@/executor/variables/resolver' import type { SerializedBlock } from '@/serializer/types' import { executeTool } from '@/tools' import type { ToolResponse } from '@/tools/types' @@ -29,6 +30,8 @@ interface ConditionEntry { id: string title: string value: string + /** Set by the resolver from the author's pre-resolution expression. */ + [CONDITION_READS_ENVIRONMENT_KEY]?: boolean } /** Verdict for a whole condition list evaluated in one function execution. */ @@ -97,32 +100,36 @@ function buildConditionScript(expressions: string[], evalContext: Record /\benvironmentVariables\b/.test(expression))) { + const readsEnvironment = conditions.some((condition) => { + const recorded = condition[CONDITION_READS_ENVIRONMENT_KEY] + return recorded ?? /\benvironmentVariables\b/.test(condition.value) + }) + if (readsEnvironment) { return { secretScope: 'all', mountedSecrets: [] } } const named = new Set() - for (const expression of expressions) { - for (const match of expression.matchAll(createEnvVarPattern())) { + for (const condition of conditions) { + for (const match of String(condition.value ?? '').matchAll(createEnvVarPattern())) { named.add(String(match[1]).trim()) } } @@ -141,11 +148,11 @@ function scopeConditionSecrets(expressions: string[]): { async function runConditionCode( ctx: ExecutionContext, code: string, - expressions: string[], + conditions: ConditionEntry[], currentNodeId?: string ): Promise { const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId) - const { secretScope, mountedSecrets } = scopeConditionSecrets(expressions) + const { secretScope, mountedSecrets } = scopeConditionSecrets(conditions) return executeTool( 'function_execute', @@ -188,14 +195,15 @@ function isTimeoutFailure(error: string | undefined): boolean { /** Evaluates the whole condition list in a single function execution. */ async function evaluateConditionList( ctx: ExecutionContext, - expressions: string[], + conditions: ConditionEntry[], evalContext: Record, currentNodeId?: string ): Promise { + const expressions = conditions.map((condition) => String(condition.value || '')) const result = await runConditionCode( ctx, buildConditionScript(expressions, evalContext), - expressions, + conditions, currentNodeId ) @@ -275,12 +283,13 @@ async function evaluateConditionList( */ async function evaluateSingleCondition( ctx: ExecutionContext, - expression: string, + condition: ConditionEntry, evalContext: Record, currentNodeId?: string ): Promise { + const expression = String(condition.value || '') const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}` - const result = await runConditionCode(ctx, code, [expression], currentNodeId) + const result = await runConditionCode(ctx, code, [condition], currentNodeId) if (!result.success) { if (result.retryable === false) { @@ -464,11 +473,9 @@ export class ConditionBlockHandler implements BlockHandler { ): Promise { if (conditions.length === 0) return null - const expressions = conditions.map((condition) => String(condition.value || '')) - let evaluation: ConditionEvaluation try { - evaluation = await evaluateConditionList(ctx, expressions, evalContext, currentNodeId) + evaluation = await evaluateConditionList(ctx, conditions, evalContext, currentNodeId) } catch (error) { if (isNonRetryableExecutionError(error)) throw error evaluation = { @@ -520,7 +527,7 @@ export class ConditionBlockHandler implements BlockHandler { try { const conditionMet = await evaluateSingleCondition( ctx, - String(condition.value || ''), + condition, evalContext, currentNodeId ) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index fb068f5c9ac..30630c47127 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -236,16 +236,50 @@ describe('VariableResolver function block inputs', () => { ) expect(result.conditions).toEqual([ - { id: 'condition-1', title: 'if', value: '123 === 123' }, - { id: 'condition-2', title: 'else if', value: 'true === true' }, + { id: 'condition-1', title: 'if', value: '123 === 123', _readsEnvironmentVariables: false }, + { + id: 'condition-2', + title: 'else if', + value: 'true === true', + _readsEnvironmentVariables: false, + }, { id: 'condition-3', title: 'else if', value: '"Bearer {{API_KEY}}" === "Bearer token"', + _readsEnvironmentVariables: false, }, ]) }) + it('records whether the author, not the trigger data, reads the environment map', async () => { + const { ctx, resolver } = createResolver() + ctx.environmentVariables = {} + const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION) + // The second branch only quotes a producer output that happens to contain the word. + const conditions = [ + { id: 'c1', title: 'if', value: `environmentVariables.FLAG === 'on'` }, + { id: 'c2', title: 'else if', value: `"" === 'x'` }, + ] + ;(ctx.blockStates as Map).set('producer', { + output: { result: 'environmentVariables.OPENAI_API_KEY' }, + executed: true, + executionTime: 0, + }) + + const result = await resolver.resolveInputs( + ctx, + conditionBlock.id, + { conditions: JSON.stringify(conditions) }, + conditionBlock + ) + + const resolvedConditions = result.conditions as Array> + expect(resolvedConditions[0]._readsEnvironmentVariables).toBe(true) + expect(resolvedConditions[1]._readsEnvironmentVariables).toBe(false) + expect(resolvedConditions[1].value).toContain('environmentVariables.OPENAI_API_KEY') + }) + it('preserves legacy condition outcomes end to end through the boundary compiler', async () => { const environmentVariables = { API_KEY: 'token', diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index fd86e881def..41fdab5bf59 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -36,6 +36,18 @@ import { import { WorkflowResolver } from '@/executor/variables/resolvers/workflow' import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types' +/** + * Marks a Condition branch whose author-written expression reads the environment map. + * + * Carried per branch because only the pre-resolution text can answer it: the handler decides + * which secrets to mount, and by the time it runs, resolved trigger data quoted inside the + * expression would read the same as the author reaching for the map. + */ +export const CONDITION_READS_ENVIRONMENT_KEY = '_readsEnvironmentVariables' + +/** The sandbox global holding the run's secrets, in whatever shape an expression reaches it. */ +const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/ + /** Key used to carry pre-resolved context variables through the inputs map. */ export const FUNCTION_BLOCK_CONTEXT_VARS_KEY = '_runtimeContextVars' /** Key used to carry display-resolved code through the function execution path. */ @@ -346,6 +358,11 @@ export class VariableResolver { const value = Reflect.get(condition, 'value') return { ...condition, + // Recorded before resolution: once values are inlined, an expression that reads + // the environment map is indistinguishable from one that merely quotes trigger + // data containing the word, and the handler decides what to mount from this. + [CONDITION_READS_ENVIRONMENT_KEY]: + typeof value === 'string' && ENVIRONMENT_MAP_IDENTIFIER.test(value), value: typeof value === 'string' ? await this.resolveTemplateWithoutConditionFormatting( @@ -1129,14 +1146,17 @@ export class VariableResolver { private canStartJavaScriptRegex( template: string, previousSignificantIndex: number, - controlHeadParenCloses: ReadonlySet + closes: { controlHeadParenCloses: ReadonlySet; regexCloseIndices: ReadonlySet } ): boolean { if (previousSignificantIndex < 0) { return true } const previous = template[previousSignificantIndex] if (previous === ')') { - return controlHeadParenCloses.has(previousSignificantIndex) + return closes.controlHeadParenCloses.has(previousSignificantIndex) + } + if (previous === '/' && closes.regexCloseIndices.has(previousSignificantIndex)) { + return false } if (JAVASCRIPT_REGEX_ALLOWED_AFTER.has(previous)) { return true @@ -1304,6 +1324,7 @@ export class VariableResolver { let lastSignificantIndex = -1 const openParenIsControlHead: boolean[] = [] const controlHeadParenCloses = new Set() + const regexCloseIndices = new Set() for (let i = 0; i < index; i++) { const char = template[i] @@ -1346,6 +1367,9 @@ export class VariableResolver { } if (char === '/' && !mode.inCharacterClass) { modes.pop() + // The literal that just closed is a value, so the next `/` divides it. + regexCloseIndices.add(i) + lastSignificantIndex = i } continue } @@ -1414,7 +1438,10 @@ export class VariableResolver { if ( !isPython && char === '/' && - this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses) + this.canStartJavaScriptRegex(template, previousSignificantIndex, { + controlHeadParenCloses, + regexCloseIndices, + }) ) { modes.push({ type: 'regex', inCharacterClass: false }) continue @@ -1480,7 +1507,10 @@ export class VariableResolver { if ( !isPython && char === '/' && - this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses) + this.canStartJavaScriptRegex(template, previousSignificantIndex, { + controlHeadParenCloses, + regexCloseIndices, + }) ) { modes.push({ type: 'regex', inCharacterClass: false }) continue From 4fb30274941463fb08ed6bcb3df3abffe4c32328 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 14:25:08 -0700 Subject: [PATCH 08/13] fix(executor): do not read a keyword-named method as a control-flow head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `p.catch(fn)` is a call whose name happens to be a keyword, and what follows its `)` is an operator rather than a statement — so the division in `p.catch(fn) / 2` opened regex mode and ran over the quotes around any reference later on that line. A control-flow head is never a property access, so the check now refuses one that follows a dot. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 17 +++++++++++++++++ apps/sim/executor/variables/resolver.ts | 12 +++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 30630c47127..32fd07e82d8 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -947,6 +947,23 @@ describe('VariableResolver function block inputs', () => { expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) }) + it('does not read a method named after a keyword as a control-flow head', async () => { + const { block, ctx, resolver } = createResolver('javascript') + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { code: `const n = params.p.catch(() => 0) / 2 + Number('')` }, + block + ) + + // `.catch(…)` is a call, so the slash after it divides — it must not open a regex that + // runs over the quotes around the reference. + expect(result.resolvedInputs.code).toContain( + `Number('' + JSON.stringify(globalThis["__blockRef_0"]) + '')` + ) + }) + it('binds a workflow variable carrying quote characters instead of splicing it into code', async () => { // A Variables block can assign trigger data at runtime, so a variable's value is not // necessarily the author's. Inlined as a literal it closed the string it landed in. diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 41fdab5bf59..0c7c18ad2bb 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -1191,7 +1191,17 @@ export class VariableResolver { while (start > 0 && this.isJavaScriptIdentifierChar(template[start - 1])) { start-- } - return CONTROL_FLOW_HEAD_KEYWORDS.has(template.slice(start, end)) + if (!CONTROL_FLOW_HEAD_KEYWORDS.has(template.slice(start, end))) { + return false + } + + // `p.catch(fn)` is a method call whose name happens to be a keyword, and what follows its + // `)` is an operator, not a statement. A control-flow head can never be a property access. + let before = start + while (before > 0 && WHITESPACE_CHAR.test(template[before - 1])) { + before-- + } + return template[before - 1] !== '.' } private matchesKeywordAt(template: string, index: number, keyword: string): boolean { From 82a80c3d5d648c598b6af28337650ab8cde31083 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 14:45:43 -0700 Subject: [PATCH 09/13] fix(executor): keep a caller from choosing which secret expands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A placeholder-bearing string stays in source so the boundary compiler can expand it, which is how a workflow variable holding `{{NAME}}` reaches its value. Any run value took that path too, so text arriving from a trigger or a loop item could name a secret and have the compiler materialize it beside the payload that named it. Only a workflow variable — a surface an author configures — keeps the inline form now; every other value binds. A block comment can also stand between a property-access dot and a keyword-named method, hiding the dot from the control-flow-head check. A comment ending there now reads as the method call it almost always is. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 22 ++++++++++++++++++++ apps/sim/executor/variables/resolver.ts | 19 ++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 32fd07e82d8..76df475908e 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -964,6 +964,28 @@ describe('VariableResolver function block inputs', () => { ) }) + it('binds a run value that names a secret instead of expanding it', async () => { + const { block, ctx, resolver } = createResolver('javascript') + ctx.workflowVariables = { + 'var-1': { id: 'var-1', name: 'authTemplate', type: 'string', value: 'Bearer {{API_KEY}}' }, + } + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { code: `const header = ''; const item = ` }, + block + ) + + // An author-configured variable keeps its placeholder in source, where the boundary + // compiler expands it; a run value naming a secret binds instead, so whoever supplies + // the text cannot pick what the compiler materializes next to it. + const code = result.resolvedInputs.code as string + expect(code).toContain('{{API_KEY}}') + expect(code).toContain('const item = globalThis["__blockRef_0"]') + expect(result.contextVariables).toEqual({ __blockRef_0: 'hello world' }) + }) + it('binds a workflow variable carrying quote characters instead of splicing it into code', async () => { // A Variables block can assign trigger data at runtime, so a variable's value is not // necessarily the author's. Inlined as a literal it closed the string it landed in. diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 0c7c18ad2bb..9b535447b85 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -687,7 +687,7 @@ export class VariableResolver { throw getNestedLargeValueMaterializationError() } - if (this.canInlineResolvedCodeLiteral(effectiveValue)) { + if (this.canInlineResolvedCodeLiteral(effectiveValue, match)) { const replacement = this.blockResolver.formatValueForBlock( effectiveValue, BlockType.FUNCTION, @@ -976,14 +976,21 @@ export class VariableResolver { * Everything else binds, which is what block outputs have always done. Inlining the rest * is what let a runtime-assigned variable or loop item carrying trigger data close the * string it landed in and run as code. + * + * The placeholder case is admitted only for a workflow variable, never for a loop item or + * any other run value. Whoever supplies the text picks which secret the compiler expands + * into the generated source, so that choice stays with the surface an author configures. */ - private canInlineResolvedCodeLiteral(value: unknown): boolean { + private canInlineResolvedCodeLiteral(value: unknown, reference: string): boolean { if (value === null || typeof value === 'number' || typeof value === 'boolean') { return true } if (typeof value !== 'string') { return false } + if (parseReferencePath(reference)[0] !== REFERENCE.PREFIX.VARIABLE) { + return false + } return createEnvVarPattern().test(value) && !/['"`$\\\n\r\u2028\u2029]/.test(value) } @@ -1196,11 +1203,17 @@ export class VariableResolver { } // `p.catch(fn)` is a method call whose name happens to be a keyword, and what follows its - // `)` is an operator, not a statement. A control-flow head can never be a property access. + // `)` is an operator, not a statement. A control-flow head can never be a property access, + // and a comment can hide the dot (`p./* c */catch(fn)`), so a comment ending here is read + // as the method call it usually is: the wrong guess there costs a division scanned as a + // regex, while this way it costs nothing a regex-free line would notice. let before = start while (before > 0 && WHITESPACE_CHAR.test(template[before - 1])) { before-- } + if (template[before - 1] === '/' && template[before - 2] === '*') { + return false + } return template[before - 1] !== '.' } From 39bb39df1d9b4c0a74cf702e37c6b80a40a264ba Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 15:01:26 -0700 Subject: [PATCH 10/13] fix(executor): step over comments instead of reading them as tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Treating a comment end as the answer was too blunt: `/* c */ if (x)` is a control-flow head, and calling it a method left a statement-position regex scanned as division — the failure the comment guard was added to prevent, moved one shape over. The scan now steps back over comments to the token that precedes them, so the dot in `p./* c */catch(fn)` is still found and a keyword after a comment is still a head. A condition's environment read is placed the same way references are: an occurrence inside a string, a template, or a regex is text and mounts nothing, while any executable read — whatever its shape — keeps the map. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 49 ++++++++++++++++++ apps/sim/executor/variables/resolver.ts | 52 +++++++++++++++----- 2 files changed, 90 insertions(+), 11 deletions(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 76df475908e..8de112fd48c 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -280,6 +280,33 @@ describe('VariableResolver function block inputs', () => { expect(resolvedConditions[1].value).toContain('environmentVariables.OPENAI_API_KEY') }) + it('counts an environment read only where it can execute', async () => { + const { ctx, resolver } = createResolver() + const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION) + const conditions = [ + // Reads, in the shapes a pattern would have to anticipate. + { id: 'c1', title: 'if', value: `environmentVariables?.FLAG === 'on'` }, + { id: 'c2', title: 'else if', value: 'Object.keys(environmentVariables).length > 0' }, + // Mentions: text, not code. + { id: 'c3', title: 'else if', value: `'environmentVariables.FLAG' === 'x'` }, + { id: 'c4', title: 'else if', value: '`environmentVariables` === "x"' }, + { id: 'c5', title: 'else if', value: `/environmentVariables/.test('x')` }, + ] + + const result = await resolver.resolveInputs( + ctx, + conditionBlock.id, + { conditions: JSON.stringify(conditions) }, + conditionBlock + ) + + expect( + (result.conditions as Array>).map( + (condition) => condition._readsEnvironmentVariables + ) + ).toEqual([true, true, false, false, false]) + }) + it('preserves legacy condition outcomes end to end through the boundary compiler', async () => { const environmentVariables = { API_KEY: 'token', @@ -947,6 +974,28 @@ describe('VariableResolver function block inputs', () => { expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) }) + it('steps over a comment rather than reading it as the preceding token', async () => { + const { block, ctx, resolver } = createResolver('javascript') + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { + code: [ + `/* lead */ if (params.a) /['"]/.test('')`, + `const n = params.p./* mid */catch(() => 0) / 2 + Number('')`, + ].join('\n'), + }, + block + ) + + // A comment before a control-flow keyword leaves it a head; one hiding a property dot + // still leaves the call a call. + const code = result.resolvedInputs.code as string + expect(code).toContain(`.test('' + JSON.stringify(globalThis["__blockRef_0"]) + '')`) + expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) + }) + it('does not read a method named after a keyword as a control-flow head', async () => { const { block, ctx, resolver } = createResolver('javascript') diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 9b535447b85..53266a98d4e 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -46,7 +46,7 @@ import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types' export const CONDITION_READS_ENVIRONMENT_KEY = '_readsEnvironmentVariables' /** The sandbox global holding the run's secrets, in whatever shape an expression reaches it. */ -const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/ +const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/g /** Key used to carry pre-resolved context variables through the inputs map. */ export const FUNCTION_BLOCK_CONTEXT_VARS_KEY = '_runtimeContextVars' @@ -362,7 +362,7 @@ export class VariableResolver { // the environment map is indistinguishable from one that merely quotes trigger // data containing the word, and the handler decides what to mount from this. [CONDITION_READS_ENVIRONMENT_KEY]: - typeof value === 'string' && ENVIRONMENT_MAP_IDENTIFIER.test(value), + typeof value === 'string' && this.readsEnvironmentMap(value), value: typeof value === 'string' ? await this.resolveTemplateWithoutConditionFormatting( @@ -1181,6 +1181,28 @@ export class VariableResolver { ) } + /** + * Whether a Condition expression reads the run's secrets off the environment map. + * + * The name is only a read where it can execute, so each occurrence is placed with the same + * scanner that decides how references are spliced — a mention inside a string, a template, + * or a regex is text and mounts nothing. No shape of the read itself is assumed: + * `environmentVariables?.FLAG` and `Object.keys(environmentVariables)` both count, because + * narrowing an expression that does reach the map would route the run down a branch the + * author did not write, silently, while admitting one too many only costs the narrowing. + */ + private readsEnvironmentMap(expression: string): boolean { + ENVIRONMENT_MAP_IDENTIFIER.lastIndex = 0 + let match = ENVIRONMENT_MAP_IDENTIFIER.exec(expression) + while (match !== null) { + if (this.getCodeStringQuoteContext(expression, match.index, 'javascript') === null) { + return true + } + match = ENVIRONMENT_MAP_IDENTIFIER.exec(expression) + } + return false + } + /** * Whether the `(` at this index opens a control-flow head rather than a value. * @@ -1204,19 +1226,27 @@ export class VariableResolver { // `p.catch(fn)` is a method call whose name happens to be a keyword, and what follows its // `)` is an operator, not a statement. A control-flow head can never be a property access, - // and a comment can hide the dot (`p./* c */catch(fn)`), so a comment ending here is read - // as the method call it usually is: the wrong guess there costs a division scanned as a - // regex, while this way it costs nothing a regex-free line would notice. - let before = start - while (before > 0 && WHITESPACE_CHAR.test(template[before - 1])) { - before-- - } - if (template[before - 1] === '/' && template[before - 2] === '*') { - return false + // and a comment can stand between the two (`p./* c */catch(fn)`), so a comment is stepped + // over rather than treated as an answer — `/* c */ if (x)` is still a head. + let before = this.skipWhitespaceBackward(template, start) + while (template[before - 1] === '/' && template[before - 2] === '*') { + const opening = template.lastIndexOf('/*', before - 2) + if (opening < 0) { + return true + } + before = this.skipWhitespaceBackward(template, opening) } return template[before - 1] !== '.' } + private skipWhitespaceBackward(template: string, index: number): number { + let cursor = index + while (cursor > 0 && WHITESPACE_CHAR.test(template[cursor - 1])) { + cursor-- + } + return cursor + } + private matchesKeywordAt(template: string, index: number, keyword: string): boolean { if (!template.startsWith(keyword, index)) { return false From 4a6ced56f0307d8c72207a25d2b8f2d39c10c3fc Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 15:11:40 -0700 Subject: [PATCH 11/13] fix(executor): take the preceding token from the scan, not from a walk back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading backwards cannot tell which characters were code: a block comment opens at its first delimiter, so `p./* a /* b */catch(fn)` defeated a search for the nearest `/*` and the call read as a control-flow head again. The scan already knows — it stepped over that comment on the way in — so the two facts the check needs, the token before the parenthesis and whether it followed a property access, are now recorded as it passes and read from there. No search back through the source, and nothing left for a comment body to imitate. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 4 + apps/sim/executor/variables/resolver.ts | 88 ++++++++++++-------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 8de112fd48c..3c1cae979b7 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -984,6 +984,9 @@ describe('VariableResolver function block inputs', () => { code: [ `/* lead */ if (params.a) /['"]/.test('')`, `const n = params.p./* mid */catch(() => 0) / 2 + Number('')`, + // A comment body may contain another opening delimiter; the comment still ends at + // the first `*/`, which only the scan that passed through it knows. + `const m = params.q./* a /* b */catch(() => 0) / 2 + Number('')`, ].join('\n'), }, block @@ -994,6 +997,7 @@ describe('VariableResolver function block inputs', () => { const code = result.resolvedInputs.code as string expect(code).toContain(`.test('' + JSON.stringify(globalThis["__blockRef_0"]) + '')`) expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) + expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_2"]) + '')`) }) it('does not read a method named after a keyword as a control-flow head', async () => { diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 53266a98d4e..6138ec3f5a2 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -1204,47 +1204,34 @@ export class VariableResolver { } /** - * Whether the `(` at this index opens a control-flow head rather than a value. + * Whether a `(` opens a control-flow head rather than a value. * * What follows the matching `)` differs entirely between the two — a statement, where a regex * literal may begin, versus an operator, where a `/` divides — and the closing parenthesis - * carries no trace of which it was. Reading the keyword in front of the opening one is what - * lets `if (a) /re/.test(b)` and `(a + b) / 2` both scan correctly. + * carries no trace of which it was. The keyword in front of the opening one is what tells + * them apart, so `if (a) /re/.test(b)` and `(a + b) / 2` both scan correctly. + * + * Both inputs come from the forward scan rather than a walk back through the source: the + * scan already knows which characters were code and which sat inside a comment, and reading + * backwards cannot recover that — the opener of `/* a /* b *\/` is its first delimiter, not + * its last, and only the scan that passed through knows the difference. */ - private opensControlFlowHead(template: string, index: number): boolean { - let end = index - while (end > 0 && WHITESPACE_CHAR.test(template[end - 1])) { - end-- - } - let start = end - while (start > 0 && this.isJavaScriptIdentifierChar(template[start - 1])) { - start-- - } - if (!CONTROL_FLOW_HEAD_KEYWORDS.has(template.slice(start, end))) { + private opensControlFlowHead( + template: string, + previousSignificantIndex: number, + precededByPropertyAccess: boolean + ): boolean { + if (precededByPropertyAccess || previousSignificantIndex < 0) { return false } - - // `p.catch(fn)` is a method call whose name happens to be a keyword, and what follows its - // `)` is an operator, not a statement. A control-flow head can never be a property access, - // and a comment can stand between the two (`p./* c */catch(fn)`), so a comment is stepped - // over rather than treated as an answer — `/* c */ if (x)` is still a head. - let before = this.skipWhitespaceBackward(template, start) - while (template[before - 1] === '/' && template[before - 2] === '*') { - const opening = template.lastIndexOf('/*', before - 2) - if (opening < 0) { - return true - } - before = this.skipWhitespaceBackward(template, opening) + if (!this.isJavaScriptIdentifierChar(template[previousSignificantIndex])) { + return false } - return template[before - 1] !== '.' - } - - private skipWhitespaceBackward(template: string, index: number): number { - let cursor = index - while (cursor > 0 && WHITESPACE_CHAR.test(template[cursor - 1])) { - cursor-- + let start = previousSignificantIndex + while (start > 0 && this.isJavaScriptIdentifierChar(template[start - 1])) { + start-- } - return cursor + return CONTROL_FLOW_HEAD_KEYWORDS.has(template.slice(start, previousSignificantIndex + 1)) } private matchesKeywordAt(template: string, index: number, keyword: string): boolean { @@ -1376,6 +1363,7 @@ export class VariableResolver { const modes: CodeScanMode[] = [{ type: 'normal' }] let lastSignificantIndex = -1 const openParenIsControlHead: boolean[] = [] + let identifierFollowsPropertyAccess = false const controlHeadParenCloses = new Set() const regexCloseIndices = new Set() @@ -1483,8 +1471,21 @@ export class VariableResolver { if (!WHITESPACE_CHAR.test(char)) { lastSignificantIndex = i } - if (char === '(') { - openParenIsControlHead.push(this.opensControlFlowHead(template, i)) + if (this.isJavaScriptIdentifierChar(char)) { + if ( + previousSignificantIndex < 0 || + !this.isJavaScriptIdentifierChar(template[previousSignificantIndex]) + ) { + identifierFollowsPropertyAccess = template[previousSignificantIndex] === '.' + } + } else if (char === '(') { + openParenIsControlHead.push( + this.opensControlFlowHead( + template, + previousSignificantIndex, + identifierFollowsPropertyAccess + ) + ) } else if (char === ')') { if (openParenIsControlHead.pop()) controlHeadParenCloses.add(i) } @@ -1552,8 +1553,21 @@ export class VariableResolver { if (!WHITESPACE_CHAR.test(char)) { lastSignificantIndex = i } - if (char === '(') { - openParenIsControlHead.push(this.opensControlFlowHead(template, i)) + if (this.isJavaScriptIdentifierChar(char)) { + if ( + previousSignificantIndex < 0 || + !this.isJavaScriptIdentifierChar(template[previousSignificantIndex]) + ) { + identifierFollowsPropertyAccess = template[previousSignificantIndex] === '.' + } + } else if (char === '(') { + openParenIsControlHead.push( + this.opensControlFlowHead( + template, + previousSignificantIndex, + identifierFollowsPropertyAccess + ) + ) } else if (char === ')') { if (openParenIsControlHead.pop()) controlHeadParenCloses.add(i) } From 522980cfa64ce16138c2a48f7033e8f415500365 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 15:21:08 -0700 Subject: [PATCH 12/13] fix(executor): divide after a postfix update `+` and `-` precede a regex as operators, but doubled they end a value, so `i++ / 2` was scanning a regex from the division and swallowing whatever quotes followed it on that line. The check now reads the pair rather than the single character; a lone `+` still admits `params.n + /re/.test(x)`. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 21 ++++++++++++++++++++ apps/sim/executor/variables/resolver.ts | 7 +++++++ 2 files changed, 28 insertions(+) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 3c1cae979b7..a7d397e3aa4 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -1000,6 +1000,27 @@ describe('VariableResolver function block inputs', () => { expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_2"]) + '')`) }) + it('divides after a postfix update rather than opening a regex', async () => { + const { block, ctx, resolver } = createResolver('javascript') + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { + code: [ + `let i = params.i; const half = i++ / 2 + Number('')`, + // The same characters as an operator still precede a regex. + `const hit = params.n + /['"]/.test('')`, + ].join('\n'), + }, + block + ) + + const code = result.resolvedInputs.code as string + expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_0"]) + '')`) + expect(code).toContain(`.test('' + JSON.stringify(globalThis["__blockRef_1"]) + '')`) + }) + it('does not read a method named after a keyword as a control-flow head', async () => { const { block, ctx, resolver } = createResolver('javascript') diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 6138ec3f5a2..65f3e38153a 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -1165,6 +1165,13 @@ export class VariableResolver { if (previous === '/' && closes.regexCloseIndices.has(previousSignificantIndex)) { return false } + // `+` and `-` precede a regex as operators but end a value when doubled: `i++ / 2` divides. + if ( + (previous === '+' || previous === '-') && + template[previousSignificantIndex - 1] === previous + ) { + return false + } if (JAVASCRIPT_REGEX_ALLOWED_AFTER.has(previous)) { return true } From ea25c3ce2cc971c273f37d2c1bac3f51245a0134 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 31 Aug 2026 15:59:05 -0700 Subject: [PATCH 13/13] fix(executor): start a new identifier at the character before it A token continues only when the character immediately before it belongs to the same token. Asking the previous *significant* character instead made a name after a line break look like a continuation, so it kept whatever property-access answer the last token had: `const seen = params.a.b` on one line left the `if` on the next carrying `b`'s, which turned the statement head into a method call and the regex after it into division. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/executor/variables/resolver.test.ts | 19 +++++++++++++++++++ apps/sim/executor/variables/resolver.ts | 16 ++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index a7d397e3aa4..f4243461260 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -1000,6 +1000,25 @@ describe('VariableResolver function block inputs', () => { expect(code).toContain(`Number('' + JSON.stringify(globalThis["__blockRef_2"]) + '')`) }) + it('starts a new identifier at a line break rather than continuing the last one', async () => { + const { block, ctx, resolver } = createResolver('javascript') + + const result = await resolver.resolveInputsForFunctionBlock( + ctx, + 'function', + { + // `if` begins a statement here; reading the previous *significant* character would + // see the `b` of `params.b` and carry its property-access answer into this token. + code: ['const seen = params.a.b', `if (seen) /['"]/.test('')`].join('\n'), + }, + block + ) + + expect(result.resolvedInputs.code).toContain( + `.test('' + JSON.stringify(globalThis["__blockRef_0"]) + '')` + ) + }) + it('divides after a postfix update rather than opening a regex', async () => { const { block, ctx, resolver } = createResolver('javascript') diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 65f3e38153a..7184e4660db 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -1479,10 +1479,10 @@ export class VariableResolver { lastSignificantIndex = i } if (this.isJavaScriptIdentifierChar(char)) { - if ( - previousSignificantIndex < 0 || - !this.isJavaScriptIdentifierChar(template[previousSignificantIndex]) - ) { + // An identifier continues only when the character right before it is part of the same + // token. Asking the previous *significant* character instead treats a name after a + // line break as a continuation and leaves it carrying the last one's answer. + if (i === 0 || !this.isJavaScriptIdentifierChar(template[i - 1])) { identifierFollowsPropertyAccess = template[previousSignificantIndex] === '.' } } else if (char === '(') { @@ -1561,10 +1561,10 @@ export class VariableResolver { lastSignificantIndex = i } if (this.isJavaScriptIdentifierChar(char)) { - if ( - previousSignificantIndex < 0 || - !this.isJavaScriptIdentifierChar(template[previousSignificantIndex]) - ) { + // An identifier continues only when the character right before it is part of the same + // token. Asking the previous *significant* character instead treats a name after a + // line break as a continuation and leaves it carrying the last one's answer. + if (i === 0 || !this.isJavaScriptIdentifierChar(template[i - 1])) { identifierFollowsPropertyAccess = template[previousSignificantIndex] === '.' } } else if (char === '(') {