Skip to content

Commit 49f568b

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

2 files changed

Lines changed: 29 additions & 12 deletions

File tree

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -254,17 +254,28 @@ describe('ConditionBlockHandler', () => {
254254
})
255255

256256
it('keeps the whole environment for a condition that reads the environment directly', async () => {
257-
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
258-
259-
const conditions = [
260-
{ id: 'cond1', title: 'if', value: 'environmentVariables.ROUTE_KEY === "beta"' },
261-
{ id: 'else1', title: 'else', value: '' },
257+
// Every shape an expression can reach the map through, including the ones a member-access
258+
// pattern would miss — narrowing one of those would route the run silently.
259+
const reads = [
260+
'environmentVariables.ROUTE_KEY === "beta"',
261+
'environmentVariables["ROUTE_KEY"] === "beta"',
262+
'environmentVariables?.ROUTE_KEY === "beta"',
263+
'Object.keys(environmentVariables).length > 0',
262264
]
263265

264-
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
266+
for (const value of reads) {
267+
mockExecuteTool.mockReset()
268+
mockExecuteTool.mockResolvedValueOnce(matchedAt(0))
269+
const conditions = [
270+
{ id: 'cond1', title: 'if', value },
271+
{ id: 'else1', title: 'else', value: '' },
272+
]
265273

266-
const [, toolParams] = mockExecuteTool.mock.calls[0]
267-
expect(toolParams.secretScope).toBe('all')
274+
await handler.execute(mockContext, mockBlock, { conditions: JSON.stringify(conditions) })
275+
276+
const [, toolParams] = mockExecuteTool.mock.calls[0]
277+
expect(toolParams.secretScope, `condition ${value}`).toBe('all')
278+
}
268279
})
269280

270281
it('should never forward collected block outputs in the request body', async () => {

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,15 @@ function buildConditionScript(expressions: string[], evalContext: Record<string,
100100
* The two scans read different text on purpose. Placeholders are read from the built script,
101101
* which is exactly what the compiler substitutes over, so a mounted set derived from anything
102102
* 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.
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.
106+
*
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.
106112
*/
107113
function scopeConditionSecrets(
108114
code: string,
@@ -111,7 +117,7 @@ function scopeConditionSecrets(
111117
secretScope: 'all' | 'selected'
112118
mountedSecrets: string[]
113119
} {
114-
if (expressions.some((expression) => /\benvironmentVariables\s*[.[]/.test(expression))) {
120+
if (expressions.some((expression) => /\benvironmentVariables\b/.test(expression))) {
115121
return { secretScope: 'all', mountedSecrets: [] }
116122
}
117123

0 commit comments

Comments
 (0)