Skip to content

Commit 47da972

Browse files
icecrasher321claude
andcommitted
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>
1 parent c28a7e4 commit 47da972

3 files changed

Lines changed: 26 additions & 21 deletions

File tree

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,13 @@ 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 () => {
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.
236239
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
237240
mockContext.blockStates.set('source-block-1', {
238-
output: { text: 'environmentVariables.OPENAI_API_KEY' },
241+
output: { text: 'environmentVariables.OPENAI_API_KEY {{OPENAI_API_KEY}}' },
239242
executed: true,
240243
executionTime: 0,
241244
} as BlockState)
@@ -248,7 +251,7 @@ describe('ConditionBlockHandler', () => {
248251
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
249252

250253
const [, toolParams] = mockExecuteTool.mock.calls[0]
251-
expect(toolParams.code).toContain('environmentVariables.OPENAI_API_KEY')
254+
expect(toolParams.code).toContain('{{OPENAI_API_KEY}}')
252255
expect(toolParams.secretScope).toBe('selected')
253256
expect(toolParams.mountedSecrets).toEqual([])
254257
})

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

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,22 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
9797
* sandbox the full environment only widens what a future defect in this path could reach —
9898
* the whole map was readable as the `environmentVariables` global.
9999
*
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: the script also carries
104-
* the source block's output as data, so scanning that too would let a payload which merely
105-
* contains the word restore the full map.
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.
106107
*
107-
* Within an expression the bare identifier is enough, rather than a member access. Narrowing
108-
* the secrets an expression can see when it does reach for the map by some shape the pattern
109-
* did not anticipate — `environmentVariables?.FLAG`, or a read through `Object.keys` — would
110-
* route the run down a branch the author did not write, silently. Matching too widely only
111-
* costs the narrowing itself, and never mounts more than this path already mounted.
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.
112114
*/
113-
function scopeConditionSecrets(
114-
code: string,
115-
expressions: string[]
116-
): {
115+
function scopeConditionSecrets(expressions: string[]): {
117116
secretScope: 'all' | 'selected'
118117
mountedSecrets: string[]
119118
} {
@@ -122,8 +121,10 @@ function scopeConditionSecrets(
122121
}
123122

124123
const named = new Set<string>()
125-
for (const match of code.matchAll(createEnvVarPattern())) {
126-
named.add(String(match[1]).trim())
124+
for (const expression of expressions) {
125+
for (const match of expression.matchAll(createEnvVarPattern())) {
126+
named.add(String(match[1]).trim())
127+
}
127128
}
128129
return { secretScope: 'selected', mountedSecrets: [...named] }
129130
}
@@ -144,7 +145,7 @@ async function runConditionCode(
144145
currentNodeId?: string
145146
): Promise<ToolResponse> {
146147
const { blockNameMapping, blockOutputSchemas } = collectBlockData(ctx, currentNodeId)
147-
const { secretScope, mountedSecrets } = scopeConditionSecrets(code, expressions)
148+
const { secretScope, mountedSecrets } = scopeConditionSecrets(expressions)
148149

149150
return executeTool(
150151
'function_execute',

apps/sim/executor/variables/resolver.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ const JAVASCRIPT_REGEX_ALLOWED_AFTER_KEYWORDS = new Set([
188188
'delete',
189189
'void',
190190
'case',
191+
'throw',
191192
'do',
192193
'else',
193194
'yield',

0 commit comments

Comments
 (0)