Skip to content

Commit d5c2b89

Browse files
icecrasher321claude
andcommitted
fix(executor): decide the environment read from the author's own text
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) <noreply@anthropic.com>
1 parent 184460a commit d5c2b89

4 files changed

Lines changed: 126 additions & 33 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,28 @@ describe('ConditionBlockHandler', () => {
256256
expect(toolParams.mountedSecrets).toEqual([])
257257
})
258258

259+
it('trusts the resolver record over the word appearing in a resolved expression', async () => {
260+
// The resolver saw the author's text before any value was inlined; the expression by now
261+
// carries trigger data, where the same word means nothing.
262+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
263+
264+
const conditions = [
265+
{
266+
id: 'cond1',
267+
title: 'if',
268+
value: `'environmentVariables.OPENAI_API_KEY' === 'x'`,
269+
_readsEnvironmentVariables: false,
270+
},
271+
{ id: 'else1', title: 'else', value: '' },
272+
]
273+
274+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
275+
276+
const [, toolParams] = mockExecuteTool.mock.calls[0]
277+
expect(toolParams.secretScope).toBe('selected')
278+
expect(toolParams.mountedSecrets).toEqual([])
279+
})
280+
259281
it('keeps the whole environment for a condition that reads the environment directly', async () => {
260282
// Every shape an expression can reach the map through, including the ones a member-access
261283
// pattern would miss — narrowing one of those would route the run silently.

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

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
extractBranchIndex,
1818
isBranchNodeId,
1919
} from '@/executor/utils/subflow-utils'
20+
import { CONDITION_READS_ENVIRONMENT_KEY } from '@/executor/variables/resolver'
2021
import type { SerializedBlock } from '@/serializer/types'
2122
import { executeTool } from '@/tools'
2223
import type { ToolResponse } from '@/tools/types'
@@ -29,6 +30,8 @@ interface ConditionEntry {
2930
id: string
3031
title: string
3132
value: string
33+
/** Set by the resolver from the author's pre-resolution expression. */
34+
[CONDITION_READS_ENVIRONMENT_KEY]?: boolean
3235
}
3336

3437
/** Verdict for a whole condition list evaluated in one function execution. */
@@ -97,32 +100,36 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
97100
* sandbox the full environment only widens what a future defect in this path could reach —
98101
* the whole map was readable as the `environmentVariables` global.
99102
*
100-
* Both scans read the expressions rather than the built script, which also carries the source
101-
* block's output as data. Reading that data would let it decide what the sandbox holds: a
102-
* payload containing the word `environmentVariables` would restore the whole map, and one
103-
* containing `{{SECRET}}` would mount that secret and have the compiler expand it into the
104-
* data — a caller choosing which secret materializes next to it. Every legitimate route to a
105-
* secret runs through an expression, including a workflow variable holding `{{NAME}}`, because
106-
* the resolver inlines that value into the expression before this runs.
103+
* Neither signal is read from the built script, which also carries the source block's output as
104+
* data. Reading that data would let it decide what the sandbox holds — a payload containing
105+
* `{{SECRET}}` would mount that secret and have the compiler expand it beside the payload.
106+
* Placeholders are therefore read from the expressions, which is where every legitimate route
107+
* to a secret passes, including a workflow variable holding `{{NAME}}`: the resolver inlines
108+
* that value into the expression before this runs.
107109
*
108-
* Within an expression the bare `environmentVariables` identifier is enough, rather than a
109-
* member access. Narrowing the secrets an expression can see when it does reach for the map by
110-
* some shape the pattern did not anticipate — `environmentVariables?.FLAG`, or a read through
111-
* `Object.keys` — would route the run down a branch the author did not write, silently.
112-
* Matching too widely only costs the narrowing itself, and never mounts more than this path
113-
* already mounted.
110+
* A direct read of the environment map is not read from the resolved expression either, for the
111+
* same reason one step further in: resolved data is quoted inside it, so a payload containing
112+
* the word would be indistinguishable from the author reaching for the map. The resolver
113+
* records the answer from the author's pre-resolution text instead. When that record is absent
114+
* — a caller that did not resolve through it — the expression is scanned as a fallback, because
115+
* narrowing a read this missed would route the run down a branch the author did not write,
116+
* silently, while matching too widely only costs the narrowing.
114117
*/
115-
function scopeConditionSecrets(expressions: string[]): {
118+
function scopeConditionSecrets(conditions: ConditionEntry[]): {
116119
secretScope: 'all' | 'selected'
117120
mountedSecrets: string[]
118121
} {
119-
if (expressions.some((expression) => /\benvironmentVariables\b/.test(expression))) {
122+
const readsEnvironment = conditions.some((condition) => {
123+
const recorded = condition[CONDITION_READS_ENVIRONMENT_KEY]
124+
return recorded ?? /\benvironmentVariables\b/.test(condition.value)
125+
})
126+
if (readsEnvironment) {
120127
return { secretScope: 'all', mountedSecrets: [] }
121128
}
122129

123130
const named = new Set<string>()
124-
for (const expression of expressions) {
125-
for (const match of expression.matchAll(createEnvVarPattern())) {
131+
for (const condition of conditions) {
132+
for (const match of String(condition.value ?? '').matchAll(createEnvVarPattern())) {
126133
named.add(String(match[1]).trim())
127134
}
128135
}
@@ -141,11 +148,11 @@ function scopeConditionSecrets(expressions: string[]): {
141148
async function runConditionCode(
142149
ctx: ExecutionContext,
143150
code: string,
144-
expressions: string[],
151+
conditions: ConditionEntry[],
145152
currentNodeId?: string
146153
): Promise<ToolResponse> {
147154
const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId)
148-
const { secretScope, mountedSecrets } = scopeConditionSecrets(expressions)
155+
const { secretScope, mountedSecrets } = scopeConditionSecrets(conditions)
149156

150157
return executeTool(
151158
'function_execute',
@@ -188,14 +195,15 @@ function isTimeoutFailure(error: string | undefined): boolean {
188195
/** Evaluates the whole condition list in a single function execution. */
189196
async function evaluateConditionList(
190197
ctx: ExecutionContext,
191-
expressions: string[],
198+
conditions: ConditionEntry[],
192199
evalContext: Record<string, unknown>,
193200
currentNodeId?: string
194201
): Promise<ConditionEvaluation> {
202+
const expressions = conditions.map((condition) => String(condition.value || ''))
195203
const result = await runConditionCode(
196204
ctx,
197205
buildConditionScript(expressions, evalContext),
198-
expressions,
206+
conditions,
199207
currentNodeId
200208
)
201209

@@ -275,12 +283,13 @@ async function evaluateConditionList(
275283
*/
276284
async function evaluateSingleCondition(
277285
ctx: ExecutionContext,
278-
expression: string,
286+
condition: ConditionEntry,
279287
evalContext: Record<string, unknown>,
280288
currentNodeId?: string
281289
): Promise<boolean> {
290+
const expression = String(condition.value || '')
282291
const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}`
283-
const result = await runConditionCode(ctx, code, [expression], currentNodeId)
292+
const result = await runConditionCode(ctx, code, [condition], currentNodeId)
284293

285294
if (!result.success) {
286295
if (result.retryable === false) {
@@ -464,11 +473,9 @@ export class ConditionBlockHandler implements BlockHandler {
464473
): Promise<ConditionEntry | null> {
465474
if (conditions.length === 0) return null
466475

467-
const expressions = conditions.map((condition) => String(condition.value || ''))
468-
469476
let evaluation: ConditionEvaluation
470477
try {
471-
evaluation = await evaluateConditionList(ctx, expressions, evalContext, currentNodeId)
478+
evaluation = await evaluateConditionList(ctx, conditions, evalContext, currentNodeId)
472479
} catch (error) {
473480
if (isNonRetryableExecutionError(error)) throw error
474481
evaluation = {
@@ -520,7 +527,7 @@ export class ConditionBlockHandler implements BlockHandler {
520527
try {
521528
const conditionMet = await evaluateSingleCondition(
522529
ctx,
523-
String(condition.value || ''),
530+
condition,
524531
evalContext,
525532
currentNodeId
526533
)

apps/sim/executor/variables/resolver.test.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,16 +236,50 @@ describe('VariableResolver function block inputs', () => {
236236
)
237237

238238
expect(result.conditions).toEqual([
239-
{ id: 'condition-1', title: 'if', value: '123 === 123' },
240-
{ id: 'condition-2', title: 'else if', value: 'true === true' },
239+
{ id: 'condition-1', title: 'if', value: '123 === 123', _readsEnvironmentVariables: false },
240+
{
241+
id: 'condition-2',
242+
title: 'else if',
243+
value: 'true === true',
244+
_readsEnvironmentVariables: false,
245+
},
241246
{
242247
id: 'condition-3',
243248
title: 'else if',
244249
value: '"Bearer {{API_KEY}}" === "Bearer token"',
250+
_readsEnvironmentVariables: false,
245251
},
246252
])
247253
})
248254

255+
it('records whether the author, not the trigger data, reads the environment map', async () => {
256+
const { ctx, resolver } = createResolver()
257+
ctx.environmentVariables = {}
258+
const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION)
259+
// The second branch only quotes a producer output that happens to contain the word.
260+
const conditions = [
261+
{ id: 'c1', title: 'if', value: `environmentVariables.FLAG === 'on'` },
262+
{ id: 'c2', title: 'else if', value: `"<producer.result>" === 'x'` },
263+
]
264+
;(ctx.blockStates as Map<string, any>).set('producer', {
265+
output: { result: 'environmentVariables.OPENAI_API_KEY' },
266+
executed: true,
267+
executionTime: 0,
268+
})
269+
270+
const result = await resolver.resolveInputs(
271+
ctx,
272+
conditionBlock.id,
273+
{ conditions: JSON.stringify(conditions) },
274+
conditionBlock
275+
)
276+
277+
const resolvedConditions = result.conditions as Array<Record<string, unknown>>
278+
expect(resolvedConditions[0]._readsEnvironmentVariables).toBe(true)
279+
expect(resolvedConditions[1]._readsEnvironmentVariables).toBe(false)
280+
expect(resolvedConditions[1].value).toContain('environmentVariables.OPENAI_API_KEY')
281+
})
282+
249283
it('preserves legacy condition outcomes end to end through the boundary compiler', async () => {
250284
const environmentVariables = {
251285
API_KEY: 'token',

apps/sim/executor/variables/resolver.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ import {
3636
import { WorkflowResolver } from '@/executor/variables/resolvers/workflow'
3737
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
3838

39+
/**
40+
* Marks a Condition branch whose author-written expression reads the environment map.
41+
*
42+
* Carried per branch because only the pre-resolution text can answer it: the handler decides
43+
* which secrets to mount, and by the time it runs, resolved trigger data quoted inside the
44+
* expression would read the same as the author reaching for the map.
45+
*/
46+
export const CONDITION_READS_ENVIRONMENT_KEY = '_readsEnvironmentVariables'
47+
48+
/** The sandbox global holding the run's secrets, in whatever shape an expression reaches it. */
49+
const ENVIRONMENT_MAP_IDENTIFIER = /\benvironmentVariables\b/
50+
3951
/** Key used to carry pre-resolved context variables through the inputs map. */
4052
export const FUNCTION_BLOCK_CONTEXT_VARS_KEY = '_runtimeContextVars'
4153
/** Key used to carry display-resolved code through the function execution path. */
@@ -346,6 +358,11 @@ export class VariableResolver {
346358
const value = Reflect.get(condition, 'value')
347359
return {
348360
...condition,
361+
// Recorded before resolution: once values are inlined, an expression that reads
362+
// the environment map is indistinguishable from one that merely quotes trigger
363+
// data containing the word, and the handler decides what to mount from this.
364+
[CONDITION_READS_ENVIRONMENT_KEY]:
365+
typeof value === 'string' && ENVIRONMENT_MAP_IDENTIFIER.test(value),
349366
value:
350367
typeof value === 'string'
351368
? await this.resolveTemplateWithoutConditionFormatting(
@@ -1129,14 +1146,17 @@ export class VariableResolver {
11291146
private canStartJavaScriptRegex(
11301147
template: string,
11311148
previousSignificantIndex: number,
1132-
controlHeadParenCloses: ReadonlySet<number>
1149+
closes: { controlHeadParenCloses: ReadonlySet<number>; regexCloseIndices: ReadonlySet<number> }
11331150
): boolean {
11341151
if (previousSignificantIndex < 0) {
11351152
return true
11361153
}
11371154
const previous = template[previousSignificantIndex]
11381155
if (previous === ')') {
1139-
return controlHeadParenCloses.has(previousSignificantIndex)
1156+
return closes.controlHeadParenCloses.has(previousSignificantIndex)
1157+
}
1158+
if (previous === '/' && closes.regexCloseIndices.has(previousSignificantIndex)) {
1159+
return false
11401160
}
11411161
if (JAVASCRIPT_REGEX_ALLOWED_AFTER.has(previous)) {
11421162
return true
@@ -1304,6 +1324,7 @@ export class VariableResolver {
13041324
let lastSignificantIndex = -1
13051325
const openParenIsControlHead: boolean[] = []
13061326
const controlHeadParenCloses = new Set<number>()
1327+
const regexCloseIndices = new Set<number>()
13071328

13081329
for (let i = 0; i < index; i++) {
13091330
const char = template[i]
@@ -1346,6 +1367,9 @@ export class VariableResolver {
13461367
}
13471368
if (char === '/' && !mode.inCharacterClass) {
13481369
modes.pop()
1370+
// The literal that just closed is a value, so the next `/` divides it.
1371+
regexCloseIndices.add(i)
1372+
lastSignificantIndex = i
13491373
}
13501374
continue
13511375
}
@@ -1414,7 +1438,10 @@ export class VariableResolver {
14141438
if (
14151439
!isPython &&
14161440
char === '/' &&
1417-
this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses)
1441+
this.canStartJavaScriptRegex(template, previousSignificantIndex, {
1442+
controlHeadParenCloses,
1443+
regexCloseIndices,
1444+
})
14181445
) {
14191446
modes.push({ type: 'regex', inCharacterClass: false })
14201447
continue
@@ -1480,7 +1507,10 @@ export class VariableResolver {
14801507
if (
14811508
!isPython &&
14821509
char === '/' &&
1483-
this.canStartJavaScriptRegex(template, previousSignificantIndex, controlHeadParenCloses)
1510+
this.canStartJavaScriptRegex(template, previousSignificantIndex, {
1511+
controlHeadParenCloses,
1512+
regexCloseIndices,
1513+
})
14841514
) {
14851515
modes.push({ type: 'regex', inCharacterClass: false })
14861516
continue

0 commit comments

Comments
 (0)