Skip to content

Commit 8e9aeb9

Browse files
fix(executor): stop resolved data from breaking out of generated condition and function code (#7300)
* fix(executor): stop resolved data from breaking out of generated code 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 `"<start.input>".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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * fix(executor): widen the condition environment read to any mention 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) <noreply@anthropic.com> * fix(executor): teach the code scanner about regex literals 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) <noreply@anthropic.com> * fix(executor): keep resolved data out of the condition secret decision 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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> * fix(executor): do not read a keyword-named method as a control-flow head `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) <noreply@anthropic.com> * fix(executor): keep a caller from choosing which secret expands 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) <noreply@anthropic.com> * fix(executor): step over comments instead of reading them as tokens 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) <noreply@anthropic.com> * fix(executor): take the preceding token from the scan, not from a walk back 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 70783dc commit 8e9aeb9

7 files changed

Lines changed: 936 additions & 56 deletions

File tree

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

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,107 @@ describe('ConditionBlockHandler', () => {
202202
)
203203
})
204204

205+
it('mounts only the secrets the condition names', async () => {
206+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
207+
208+
const conditions = [
209+
{ id: 'cond1', title: 'if', value: '"{{ROUTE_KEY}}" === "beta"' },
210+
{ id: 'else1', title: 'else', value: '' },
211+
]
212+
213+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
214+
215+
const [, toolParams] = mockExecuteTool.mock.calls[0]
216+
expect(toolParams.secretScope).toBe('selected')
217+
expect(toolParams.mountedSecrets).toEqual(['ROUTE_KEY'])
218+
})
219+
220+
it('denies every secret to a condition that names none', async () => {
221+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
222+
223+
const conditions = [
224+
{ id: 'cond1', title: 'if', value: 'context.value > 5' },
225+
{ id: 'else1', title: 'else', value: '' },
226+
]
227+
228+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
229+
230+
const [, toolParams] = mockExecuteTool.mock.calls[0]
231+
expect(toolParams.secretScope).toBe('selected')
232+
expect(toolParams.mountedSecrets).toEqual([])
233+
})
234+
235+
it('does not let resolved data decide which secrets the sandbox holds', async () => {
236+
// The script carries the source block's output as data. Reading that data for either
237+
// signal would let a caller pick what materializes beside it — the whole map by naming
238+
// the global, or one secret by naming its placeholder.
239+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
240+
mockContext.blockStates.set('source-block-1', {
241+
output: { text: 'environmentVariables.OPENAI_API_KEY {{OPENAI_API_KEY}}' },
242+
executed: true,
243+
executionTime: 0,
244+
} as BlockState)
245+
246+
const conditions = [
247+
{ id: 'cond1', title: 'if', value: `context.text === 'x'` },
248+
{ id: 'else1', title: 'else', value: '' },
249+
]
250+
251+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
252+
253+
const [, toolParams] = mockExecuteTool.mock.calls[0]
254+
expect(toolParams.code).toContain('{{OPENAI_API_KEY}}')
255+
expect(toolParams.secretScope).toBe('selected')
256+
expect(toolParams.mountedSecrets).toEqual([])
257+
})
258+
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+
281+
it('keeps the whole environment for a condition that reads the environment directly', async () => {
282+
// Every shape an expression can reach the map through, including the ones a member-access
283+
// pattern would miss — narrowing one of those would route the run silently.
284+
const reads = [
285+
'environmentVariables.ROUTE_KEY === "beta"',
286+
'environmentVariables["ROUTE_KEY"] === "beta"',
287+
'environmentVariables?.ROUTE_KEY === "beta"',
288+
'Object.keys(environmentVariables).length > 0',
289+
]
290+
291+
for (const value of reads) {
292+
mockExecuteTool.mockReset()
293+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
294+
const conditions = [
295+
{ id: 'cond1', title: 'if', value },
296+
{ id: 'else1', title: 'else', value: '' },
297+
]
298+
299+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
300+
301+
const [, toolParams] = mockExecuteTool.mock.calls[0]
302+
expect(toolParams.secretScope, `condition ${value}`).toBe('all')
303+
}
304+
})
305+
205306
it('should never forward collected block outputs in the request body', async () => {
206307
mockCollectBlockData.mockReturnValueOnce({
207308
blockData: { 'huge-block': { payload: 'x'.repeat(1024) } },

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

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ import type { BlockOutput } from '@/blocks/types'
1010
import { BlockType, DEFAULTS, EDGE } from '@/executor/constants'
1111
import type { BlockHandler, ExecutionContext } from '@/executor/types'
1212
import { collectBlockData } from '@/executor/utils/block-data'
13+
import { createEnvVarPattern } from '@/executor/utils/reference-validation'
1314
import {
1415
buildBranchNodeId,
1516
extractBaseBlockId,
1617
extractBranchIndex,
1718
isBranchNodeId,
1819
} from '@/executor/utils/subflow-utils'
20+
import { CONDITION_READS_ENVIRONMENT_KEY } from '@/executor/variables/resolver'
1921
import type { SerializedBlock } from '@/serializer/types'
2022
import { executeTool } from '@/tools'
2123
import type { ToolResponse } from '@/tools/types'
@@ -28,6 +30,8 @@ interface ConditionEntry {
2830
id: string
2931
title: string
3032
value: string
33+
/** Set by the resolver from the author's pre-resolution expression. */
34+
[CONDITION_READS_ENVIRONMENT_KEY]?: boolean
3135
}
3236

3337
/** Verdict for a whole condition list evaluated in one function execution. */
@@ -88,6 +92,50 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
8892
].join('\n')
8993
}
9094

95+
/**
96+
* Narrows the secrets a condition evaluation can read to the ones its script names.
97+
*
98+
* A condition reaches a secret by writing `{{NAME}}`, which the execution-boundary compiler
99+
* binds. Nothing else in the script needs the workspace's other secrets, so handing the
100+
* sandbox the full environment only widens what a future defect in this path could reach —
101+
* the whole map was readable as the `environmentVariables` global.
102+
*
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.
109+
*
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.
117+
*/
118+
function scopeConditionSecrets(conditions: ConditionEntry[]): {
119+
secretScope: 'all' | 'selected'
120+
mountedSecrets: string[]
121+
} {
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) {
127+
return { secretScope: 'all', mountedSecrets: [] }
128+
}
129+
130+
const named = new Set<string>()
131+
for (const condition of conditions) {
132+
for (const match of String(condition.value ?? '').matchAll(createEnvVarPattern())) {
133+
named.add(String(match[1]).trim())
134+
}
135+
}
136+
return { secretScope: 'selected', mountedSecrets: [...named] }
137+
}
138+
91139
/**
92140
* Runs condition code through the shared function execution boundary.
93141
*
@@ -100,15 +148,19 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
100148
async function runConditionCode(
101149
ctx: ExecutionContext,
102150
code: string,
151+
conditions: ConditionEntry[],
103152
currentNodeId?: string
104153
): Promise<ToolResponse> {
105154
const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId)
155+
const { secretScope, mountedSecrets } = scopeConditionSecrets(conditions)
106156

107157
return executeTool(
108158
'function_execute',
109159
{
110160
code,
111161
timeout: CONDITION_TIMEOUT_MS,
162+
secretScope,
163+
mountedSecrets,
112164
envVars: normalizeStringRecord(ctx.environmentVariables),
113165
workflowVariables: normalizeWorkflowVariables(ctx.workflowVariables),
114166
blockData: {},
@@ -143,13 +195,15 @@ function isTimeoutFailure(error: string | undefined): boolean {
143195
/** Evaluates the whole condition list in a single function execution. */
144196
async function evaluateConditionList(
145197
ctx: ExecutionContext,
146-
expressions: string[],
198+
conditions: ConditionEntry[],
147199
evalContext: Record<string, unknown>,
148200
currentNodeId?: string
149201
): Promise<ConditionEvaluation> {
202+
const expressions = conditions.map((condition) => String(condition.value || ''))
150203
const result = await runConditionCode(
151204
ctx,
152205
buildConditionScript(expressions, evalContext),
206+
conditions,
153207
currentNodeId
154208
)
155209

@@ -229,12 +283,13 @@ async function evaluateConditionList(
229283
*/
230284
async function evaluateSingleCondition(
231285
ctx: ExecutionContext,
232-
expression: string,
286+
condition: ConditionEntry,
233287
evalContext: Record<string, unknown>,
234288
currentNodeId?: string
235289
): Promise<boolean> {
290+
const expression = String(condition.value || '')
236291
const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}`
237-
const result = await runConditionCode(ctx, code, currentNodeId)
292+
const result = await runConditionCode(ctx, code, [condition], currentNodeId)
238293

239294
if (!result.success) {
240295
if (result.retryable === false) {
@@ -418,11 +473,9 @@ export class ConditionBlockHandler implements BlockHandler {
418473
): Promise<ConditionEntry | null> {
419474
if (conditions.length === 0) return null
420475

421-
const expressions = conditions.map((condition) => String(condition.value || ''))
422-
423476
let evaluation: ConditionEvaluation
424477
try {
425-
evaluation = await evaluateConditionList(ctx, expressions, evalContext, currentNodeId)
478+
evaluation = await evaluateConditionList(ctx, conditions, evalContext, currentNodeId)
426479
} catch (error) {
427480
if (isNonRetryableExecutionError(error)) throw error
428481
evaluation = {
@@ -474,7 +527,7 @@ export class ConditionBlockHandler implements BlockHandler {
474527
try {
475528
const conditionMet = await evaluateSingleCondition(
476529
ctx,
477-
String(condition.value || ''),
530+
condition,
478531
evalContext,
479532
currentNodeId
480533
)

apps/sim/executor/utils/code-formatting.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,36 @@ export function formatLiteralForCode(value: unknown, language: 'javascript' | 'p
4646
}
4747
return JSON.stringify(value)
4848
}
49+
50+
/**
51+
* Escapes text so it cannot terminate whatever JavaScript literal it is spliced into.
52+
*
53+
* Condition expressions are user-authored JavaScript into which resolved references are
54+
* inlined as source, so the author's quoting — not this module's — decides which string
55+
* context the value lands in. Escaping only the quote the emitted literal opens leaves
56+
* `"`, a backtick, `${`, and `/` live, and `"<start.input>".includes('urgent')` then lets
57+
* trigger data close the author's string and run as code in the condition sandbox.
58+
*
59+
* So every terminator of every JavaScript string context is escaped, not just the one the
60+
* emitted literal opens. `\"`, `` \` ``, `\$`, and `\/` are identity escapes in JavaScript,
61+
* so the value a condition compares is byte-identical to what it was before — this is a
62+
* syntax guard, not a value transform.
63+
*
64+
* JavaScript only: `\$`, `` \` `` and `\/` are not identity escapes in Python, where they
65+
* would change the value. Code blocks bind their values as runtime context variables
66+
* instead of splicing them, which is why they need no escaping in any language.
67+
*/
68+
export function escapeInertStringContent(value: string): string {
69+
return value
70+
.replace(/\\/g, '\\\\')
71+
.replace(/['"`$/]/g, '\\$&')
72+
.replace(/\n/g, '\\n')
73+
.replace(/\r/g, '\\r')
74+
.replace(/\u2028/g, '\\u2028')
75+
.replace(/\u2029/g, '\\u2029')
76+
}
77+
78+
/** Wraps {@link escapeInertStringContent} output as a complete JavaScript string literal. */
79+
export function formatInertStringLiteral(value: string, quote: '"' | "'" = "'"): string {
80+
return `${quote}${escapeInertStringContent(value)}${quote}`
81+
}

0 commit comments

Comments
 (0)