Skip to content

Commit a14345e

Browse files
icecrasher321claude
andcommitted
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>
1 parent b19553e commit a14345e

7 files changed

Lines changed: 304 additions & 46 deletions

File tree

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,50 @@ 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('keeps the whole environment for a condition that reads the environment directly', async () => {
236+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
237+
238+
const conditions = [
239+
{ id: 'cond1', title: 'if', value: 'environmentVariables.ROUTE_KEY === "beta"' },
240+
{ id: 'else1', title: 'else', value: '' },
241+
]
242+
243+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
244+
245+
const [, toolParams] = mockExecuteTool.mock.calls[0]
246+
expect(toolParams.secretScope).toBe('all')
247+
})
248+
205249
it('should never forward collected block outputs in the request body', async () => {
206250
mockCollectBlockData.mockReturnValueOnce({
207251
blockData: { 'huge-block': { payload: 'x'.repeat(1024) } },

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ 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,
@@ -88,6 +89,34 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
8889
].join('\n')
8990
}
9091

92+
/**
93+
* Narrows the secrets a condition evaluation can read to the ones its script names.
94+
*
95+
* A condition reaches a secret by writing `{{NAME}}`, which the execution-boundary
96+
* compiler binds. Nothing else in the script needs the workspace's other secrets, so
97+
* handing the sandbox the full environment only widens what a future defect in this
98+
* path could reach — the whole map was readable as the `environmentVariables` global.
99+
*
100+
* Scanning the built script rather than the expressions covers the batched and
101+
* per-branch scripts with one rule, and mounts exactly the set the compiler could
102+
* substitute. A script that reads the `environmentVariables` global directly keeps the
103+
* full map: that access is undocumented for conditions but costs nothing to preserve.
104+
*/
105+
function scopeConditionSecrets(code: string): {
106+
secretScope: 'all' | 'selected'
107+
mountedSecrets: string[]
108+
} {
109+
if (/\benvironmentVariables\b/.test(code)) {
110+
return { secretScope: 'all', mountedSecrets: [] }
111+
}
112+
113+
const named = new Set<string>()
114+
for (const match of code.matchAll(createEnvVarPattern())) {
115+
named.add(String(match[1]).trim())
116+
}
117+
return { secretScope: 'selected', mountedSecrets: [...named] }
118+
}
119+
91120
/**
92121
* Runs condition code through the shared function execution boundary.
93122
*
@@ -103,12 +132,15 @@ async function runConditionCode(
103132
currentNodeId?: string
104133
): Promise<ToolResponse> {
105134
const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId)
135+
const { secretScope, mountedSecrets } = scopeConditionSecrets(code)
106136

107137
return executeTool(
108138
'function_execute',
109139
{
110140
code,
111141
timeout: CONDITION_TIMEOUT_MS,
142+
secretScope,
143+
mountedSecrets,
112144
envVars: normalizeStringRecord(ctx.environmentVariables),
113145
workflowVariables: normalizeWorkflowVariables(ctx.workflowVariables),
114146
blockData: {},

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+
}

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

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,43 @@ async function resolveConditionExpression(
144144
return (result.conditions as Array<{ value: string }>)[0].value
145145
}
146146

147+
/** Resolves one condition expression against a producer output an attacker supplied. */
148+
async function resolveConditionWithBlockOutput(value: string, result: unknown): Promise<string> {
149+
const { ctx, resolver, state } = createResolver()
150+
state.setBlockOutput('producer', { result } as never)
151+
const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION)
152+
const resolved = await resolver.resolveInputs(
153+
ctx,
154+
conditionBlock.id,
155+
{ conditions: JSON.stringify([{ id: 'condition-1', title: 'if', value }]) },
156+
conditionBlock
157+
)
158+
return (resolved.conditions as Array<{ value: string }>)[0].value
159+
}
160+
161+
const INJECTION_CANARY = '__conditionInjectionCanary'
162+
163+
/**
164+
* Evaluates a resolved expression inside the same `Boolean(...)` wrapper the handler builds,
165+
* reporting both the branch verdict and whether anything the resolved data carried executed.
166+
*/
167+
function runResolvedCondition(expression: string): { matched: boolean; injected: boolean } {
168+
Reflect.set(globalThis, INJECTION_CANARY, 'not-executed')
169+
try {
170+
const matched = Boolean(
171+
new Function(`const context = {};\nreturn Boolean(\n${expression}\n)`)()
172+
)
173+
return { matched, injected: Reflect.get(globalThis, INJECTION_CANARY) !== 'not-executed' }
174+
} catch {
175+
return {
176+
matched: false,
177+
injected: Reflect.get(globalThis, INJECTION_CANARY) !== 'not-executed',
178+
}
179+
} finally {
180+
Reflect.deleteProperty(globalThis, INJECTION_CANARY)
181+
}
182+
}
183+
147184
/**
148185
* Completes the round trip a condition actually takes: resolver, then the execution-boundary
149186
* compiler, then evaluation of the same `Boolean(...)` wrapper `condition-handler.ts` builds.
@@ -255,6 +292,60 @@ describe('VariableResolver function block inputs', () => {
255292
).resolves.toBe(true)
256293
})
257294

295+
it('stops trigger data from breaking out of a quoted condition reference', async () => {
296+
// Every quoting an author can put around a reference. The author picks the context;
297+
// the resolved value must be data in all of them, not just the one it wraps itself in.
298+
const quotings = [
299+
`"<producer.result>".includes('urgent')`,
300+
'"<producer.result>" === "admin"',
301+
'`<producer.result>`.length > 0',
302+
'/<producer.result>/.test("x")',
303+
`<producer.result> === 'admin'`,
304+
]
305+
const payloads = [
306+
`" + (globalThis.${INJECTION_CANARY} = "ran") + "`,
307+
`\${(globalThis.${INJECTION_CANARY} = "ran")}`,
308+
`' + (globalThis.${INJECTION_CANARY} = "ran") + '`,
309+
`/ + (globalThis.${INJECTION_CANARY} = "ran") + /`,
310+
]
311+
312+
for (const value of quotings) {
313+
for (const payload of payloads) {
314+
const expression = await resolveConditionWithBlockOutput(value, payload)
315+
expect(
316+
runResolvedCondition(expression).injected,
317+
`condition ${value} executed trigger data: ${expression}`
318+
).toBe(false)
319+
}
320+
}
321+
})
322+
323+
it('stops a trigger-supplied object from closing the string it is quoted inside', async () => {
324+
// JSON's own structural quotes close the author's string, and the key is the attacker's.
325+
const expression = await resolveConditionWithBlockOutput('"<producer.result>" === "{}"', {
326+
[`+(globalThis.${INJECTION_CANARY}=1)+`]: 1,
327+
})
328+
expect(runResolvedCondition(expression).injected).toBe(false)
329+
})
330+
331+
it('keeps quoted and bare condition references comparing what they compared before', async () => {
332+
const cases: Array<{ value: string; result: unknown; expected: boolean }> = [
333+
{ value: `<producer.result> === 'urgent'`, result: 'urgent', expected: true },
334+
{ value: `<producer.result> === 'urgent'`, result: 'other', expected: false },
335+
{ value: `"<producer.result>".includes('urgent')`, result: 'urgent ticket', expected: true },
336+
{ value: `"<producer.result>".includes('urgent')`, result: 'calm ticket', expected: false },
337+
{ value: '`<producer.result>`.length > 3', result: 'hello', expected: true },
338+
{ value: `<producer.result> === 'a"b'`, result: 'a"b', expected: true },
339+
{ value: '<producer.result> === `a$b/c`', result: 'a$b/c', expected: true },
340+
{ value: `<producer.result>.count === 2`, result: { count: 2 }, expected: true },
341+
]
342+
343+
for (const { value, result, expected } of cases) {
344+
const expression = await resolveConditionWithBlockOutput(value, result)
345+
expect(runResolvedCondition(expression).matched, `condition ${value}`).toBe(expected)
346+
}
347+
})
348+
258349
it('compares a bare string placeholder instead of throwing a reference error', async () => {
259350
await expect(
260351
evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'alice' })
@@ -719,6 +810,28 @@ describe('VariableResolver function block inputs', () => {
719810
expect(result.contextVariables).toEqual({ __blockRef_0: ref })
720811
})
721812

813+
it('binds a workflow variable carrying quote characters instead of splicing it into code', async () => {
814+
// A Variables block can assign trigger data at runtime, so a variable's value is not
815+
// necessarily the author's. Inlined as a literal it closed the string it landed in.
816+
const { block, ctx, resolver } = createResolver('javascript')
817+
const payload = `' + (globalThis.__functionInjection = 1) + '`
818+
ctx.workflowVariables = {
819+
'var-1': { id: 'var-1', name: 'userinput', type: 'string', value: payload },
820+
}
821+
822+
const result = await resolver.resolveInputsForFunctionBlock(
823+
ctx,
824+
'function',
825+
{ code: `const x = '<variable.userinput>'; return x` },
826+
block
827+
)
828+
829+
expect(result.resolvedInputs.code).toBe(
830+
`const x = '' + JSON.stringify(globalThis["__blockRef_0"]) + ''; return x`
831+
)
832+
expect(result.contextVariables).toEqual({ __blockRef_0: payload })
833+
})
834+
722835
it('rewrites whole manifest workflow variables to lazy JavaScript array reads', async () => {
723836
const { block, ctx, resolver } = createResolver('javascript')
724837
const manifest = createTestManifest()
@@ -791,8 +904,11 @@ describe('VariableResolver function block inputs', () => {
791904
['0', 'key'],
792905
expect.objectContaining({ allowLargeValueRefs: true })
793906
)
794-
expect(result.resolvedInputs.code).toBe('return "SIM-0"')
795-
expect(result.contextVariables).toEqual({})
907+
// The navigated element binds like any other resolved value; what must not appear is
908+
// the manifest, or the array it stands for.
909+
expect(result.resolvedInputs.code).toBe('return globalThis["__blockRef_0"]')
910+
expect(result.displayInputs.code).toBe('return "SIM-0"')
911+
expect(result.contextVariables).toEqual({ __blockRef_0: 'SIM-0' })
796912
})
797913

798914
it('resolves named loop result bracket paths in function code', async () => {

0 commit comments

Comments
 (0)