Skip to content

Commit e4bf282

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

4 files changed

Lines changed: 88 additions & 20 deletions

File tree

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,27 @@ describe('ConditionBlockHandler', () => {
232232
expect(toolParams.mountedSecrets).toEqual([])
233233
})
234234

235+
it('does not let resolved data widen the mounted secrets by naming the environment', async () => {
236+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
237+
mockContext.blockStates.set('source-block-1', {
238+
output: { text: 'environmentVariables.OPENAI_API_KEY' },
239+
executed: true,
240+
executionTime: 0,
241+
} as BlockState)
242+
243+
const conditions = [
244+
{ id: 'cond1', title: 'if', value: `context.text === 'x'` },
245+
{ id: 'else1', title: 'else', value: '' },
246+
]
247+
248+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
249+
250+
const [, toolParams] = mockExecuteTool.mock.calls[0]
251+
expect(toolParams.code).toContain('environmentVariables.OPENAI_API_KEY')
252+
expect(toolParams.secretScope).toBe('selected')
253+
expect(toolParams.mountedSecrets).toEqual([])
254+
})
255+
235256
it('keeps the whole environment for a condition that reads the environment directly', async () => {
236257
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
237258

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

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,26 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
9292
/**
9393
* Narrows the secrets a condition evaluation can read to the ones its script names.
9494
*
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.
95+
* A condition reaches a secret by writing `{{NAME}}`, which the execution-boundary compiler
96+
* binds. Nothing else in the script needs the workspace's other secrets, so handing the
97+
* sandbox the full environment only widens what a future defect in this path could reach —
98+
* the whole map was readable as the `environmentVariables` global.
9999
*
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.
100+
* The two scans read different text on purpose. Placeholders are read from the built script,
101+
* which is exactly what the compiler substitutes over, so a mounted set derived from anything
102+
* narrower would silently stop resolving a placeholder the compiler still expands. The direct
103+
* `environmentVariables` read is looked for in the expressions alone, and only as a member
104+
* access: the script also carries the source block's output as data, so scanning it would let
105+
* a payload that merely contains the word restore the full map.
104106
*/
105-
function scopeConditionSecrets(code: string): {
107+
function scopeConditionSecrets(
108+
code: string,
109+
expressions: string[]
110+
): {
106111
secretScope: 'all' | 'selected'
107112
mountedSecrets: string[]
108113
} {
109-
if (/\benvironmentVariables\b/.test(code)) {
114+
if (expressions.some((expression) => /\benvironmentVariables\s*[.[]/.test(expression))) {
110115
return { secretScope: 'all', mountedSecrets: [] }
111116
}
112117

@@ -129,10 +134,11 @@ function scopeConditionSecrets(code: string): {
129134
async function runConditionCode(
130135
ctx: ExecutionContext,
131136
code: string,
137+
expressions: string[],
132138
currentNodeId?: string
133139
): Promise<ToolResponse> {
134140
const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId)
135-
const { secretScope, mountedSecrets } = scopeConditionSecrets(code)
141+
const { secretScope, mountedSecrets } = scopeConditionSecrets(code, expressions)
136142

137143
return executeTool(
138144
'function_execute',
@@ -182,6 +188,7 @@ async function evaluateConditionList(
182188
const result = await runConditionCode(
183189
ctx,
184190
buildConditionScript(expressions, evalContext),
191+
expressions,
185192
currentNodeId
186193
)
187194

@@ -266,7 +273,7 @@ async function evaluateSingleCondition(
266273
currentNodeId?: string
267274
): Promise<boolean> {
268275
const code = `const context = ${JSON.stringify(evalContext)};\nreturn ${buildBooleanTest(expression)}`
269-
const result = await runConditionCode(ctx, code, currentNodeId)
276+
const result = await runConditionCode(ctx, code, [expression], currentNodeId)
270277

271278
if (!result.success) {
272279
if (result.retryable === false) {

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,41 @@ describe('VariableResolver function block inputs', () => {
328328
expect(runResolvedCondition(expression).injected).toBe(false)
329329
})
330330

331+
it('stops a trigger-supplied object from escaping wherever the quote scanner mis-reads', async () => {
332+
// A regex literal is not tracked by the quote scanner, and a quote inside one
333+
// desynchronizes it for everything that follows, so the emitted object must be inert
334+
// whichever context the scanner reports.
335+
const payloads = [
336+
{ [`+(globalThis.${INJECTION_CANARY}=1)+`]: 1 },
337+
{ forged: `/ + (globalThis.${INJECTION_CANARY}=1) + /` },
338+
{ closed: `" + (globalThis.${INJECTION_CANARY}=1) + "` },
339+
]
340+
const quotings = [
341+
'/<producer.result>/.test("x")',
342+
`/['"]/.test('a') && <producer.result>.count === 2`,
343+
`/['"]/.test('a') && "<producer.result>" === "{}"`,
344+
'<producer.result>.count === 2',
345+
]
346+
347+
for (const value of quotings) {
348+
for (const payload of payloads) {
349+
const expression = await resolveConditionWithBlockOutput(value, payload)
350+
expect(
351+
runResolvedCondition(expression).injected,
352+
`condition ${value} executed object data: ${expression}`
353+
).toBe(false)
354+
}
355+
}
356+
})
357+
358+
it('keeps navigating an object reference the scanner reports as unquoted', async () => {
359+
const expression = await resolveConditionWithBlockOutput('<producer.result>.count === 2', {
360+
count: 2,
361+
note: `a "quoted" / slashed ' value`,
362+
})
363+
expect(runResolvedCondition(expression).matched).toBe(true)
364+
})
365+
331366
it('keeps quoted and bare condition references comparing what they compared before', async () => {
332367
const cases: Array<{ value: string; result: unknown; expected: boolean }> = [
333368
{ value: `<producer.result> === 'urgent'`, result: 'urgent', expected: true },

apps/sim/executor/variables/resolver.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,17 +1590,22 @@ export class VariableResolver {
15901590
/**
15911591
* Renders a resolved object for a Condition expression.
15921592
*
1593-
* In expression position the author is comparing or navigating the object itself, so the
1594-
* JSON has to stay a literal. Inside a quoted string the same text is data, and its own
1595-
* structural quotes would close the author's string — `"<start.body>" === "{}"` with a
1596-
* crafted key emits `"{"+attackerCode()+":1}"`, which parses as concatenation and runs.
1597-
* Escaping the text there keeps it inside the string the author opened, which is also the
1598-
* only reading of a quoted object reference that is not a syntax error.
1593+
* Inside a quoted string the object is data, so its JSON is escaped to stay inside the
1594+
* string the author opened: raw, the JSON's own structural quotes close that string, and
1595+
* `"<start.body>" === "{}"` with a crafted key emits `"{"+attackerCode()+":1}"`, which
1596+
* parses as concatenation and runs.
1597+
*
1598+
* Everywhere else the object is parsed at runtime rather than spliced as source. The value
1599+
* is identical to the object literal it replaces, but the payload is a fully escaped
1600+
* literal, so a crafted key can neither close a string nor forge a regex delimiter — which
1601+
* matters because the emitted form must be safe even when the quote scanner reads the
1602+
* surrounding context wrongly, and a quote inside a regex literal is enough to do that.
1603+
* Splicing raw JSON would make that heuristic load-bearing for injection.
15991604
*/
16001605
private formatConditionJson(value: object, template: string, matchIndex: number): string {
1601-
const json = JSON.stringify(value)
1606+
const escaped = escapeInertStringContent(JSON.stringify(value))
16021607
const quoteContext = this.getCodeStringQuoteContext(template, matchIndex, 'javascript')
1603-
return quoteContext === null ? json : escapeInertStringContent(json)
1608+
return quoteContext === null ? `JSON.parse('${escaped}')` : escaped
16041609
}
16051610

16061611
private async resolveReference(reference: string, context: ResolutionContext): Promise<any> {

0 commit comments

Comments
 (0)