Skip to content

Commit d2fbd20

Browse files
icecrasher321claude
andcommitted
fix(condition): stop a secret value from breaking or forging a condition
Condition expressions pasted every environment variable value into the expression as source. Block references in the same expression go through a proper escape and get quoted; env vars went through neither. That left three defects: - A bare string placeholder was a SyntaxError. `{{NAME}} === 'alice'` resolved to `alice === 'alice'`, so the form the Function block docs recommend could not be used here at all. - Ordinary data broke the block. An apostrophe (`O'Brien`) or a newline in a legitimate value produced unparseable source and failed the run. - The quoted form was injectable. A value of `x' || true || '` turned `'{{NAME}}' === 'bob'` into `'x' || true || '' === 'bob'`, forging a true branch out of a comparison that should be false. Inline only structurally inert literals — numbers, booleans, and null, with optional space/tab padding. Every other value keeps its `{{NAME}}` placeholder and is bound as a string by the execution-boundary compiler, the same one Function blocks and Custom Tools already use. Legacy outcomes are preserved. `{{COUNT}} === 3` and `{{ENABLED}} === true` still compare as literals, and an embedded `"Bearer {{API_KEY}}"` still compares equal — now via compiled concatenation rather than a pasted value. Padding is admitted rather than trimmed so the inlined text stays byte-identical to the stored value, which is what keeps a padded number correct both bare and quoted. A resolved secret also no longer travels to the execution boundary inside the condition source. The one deliberate behavior change: a value whose text is itself a quoted JS literal (a secret stored as `'foo'`, a plausible workaround for the bare-string SyntaxError) now compares as the 5-character string rather than as source. That form is the injectable one, so it cannot be kept. Docs: state the placeholder type contract, which was described mechanically but never in terms of what a reader gets. `{{KEY}}` in Function and Custom Tool code always evaluates to a string, so a bare `if ({{FLAG}})` is always true and a list has to be stored as JSON. This is what a customer hit after the resolver lift in #6247 moved Function blocks off source inlining. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a7115e8 commit d2fbd20

5 files changed

Lines changed: 178 additions & 3 deletions

File tree

apps/docs/content/docs/en/workflows/blocks/condition.mdx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ Reference an earlier output inside an expression with a [connection tag](/workfl
3131
<user.email>.endsWith('@company.com') && <user.plan> === 'pro'
3232
```
3333

34+
Read an [environment variable](/workflows/variables#environment-variables) with `{{KEY}}`:
35+
36+
```javascript
37+
{{MAX_RETRIES}} === 3
38+
{{FEATURE_ON}} === true
39+
{{TIER}} === 'pro'
40+
```
41+
42+
Numbers, booleans, and `null` compare as literals. Every other value is bound as a string, so an apostrophe, quote, or newline inside a secret cannot change what the expression means.
43+
3444
<Callout type="info">
3545
If an expression throws, for example because it reads a field that is not there, the block errors and the run follows the [error path](/workflows/connections) if one is connected. Guard missing values with optional chaining (`?.`) or a null check.
3646
</Callout>

apps/docs/content/docs/en/workflows/blocks/function.mdx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,25 @@ const apiKey = {{API_KEY}};
5858

5959
Existing quoted and embedded forms are also supported, including `"{{API_KEY}}"`, `"Bearer {{API_KEY}}"`, and placeholders in template literals. Function and Custom Tool code use the same compiler at the execution boundary. It binds the secret separately from the source instead of pasting plaintext into your code, so quotes, backslashes, newlines, and string values such as `"123"` and `"true"` retain their exact contents and do not become JavaScript or Python literals of another type.
6060

61+
### Placeholders are always strings
62+
63+
A placeholder is bound as a value, never parsed as source. That is what keeps a secret from running as code — but it also means `{{KEY}}` always evaluates to a **string**, whatever the value looks like. Convert it when your code needs another type:
64+
65+
```javascript
66+
const retries = Number({{MAX_RETRIES}});
67+
const enabled = {{FEATURE_ON}} === 'true';
68+
const patterns = JSON.parse({{PATTERN_LIST}});
69+
```
70+
71+
In Python, use `int()`, `== "true"`, and `json.loads()` the same way.
72+
73+
Two cases are easy to miss:
74+
75+
- **Booleans.** A bare `if ({{FEATURE_ON}})` is always true, because the string `"false"` is truthy. Compare against `'true'` instead.
76+
- **Lists and objects.** Store the value as JSON so you can parse it back. `["^[A-Z]{2}-\\d{4}$", "^\\d{7,15}$"]` becomes an array; a JavaScript array of regex literals does not. It arrives as one long string, and iterating it walks character by character.
77+
78+
[Condition](/workflows/blocks/condition) blocks differ in one way: a number, boolean, or `null` value compares as a literal there, so `{{MAX_RETRIES}} === 3` is true. Every other value is a string, exactly as it is here.
79+
6180
JavaScript regex literals can contain a placeholder:
6281

6382
```javascript

apps/docs/content/docs/en/workflows/variables.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ Reference them with double curly braces in any block field, including Agent syst
8585
Environment variable names must start with a letter or underscore and contain only letters, numbers, and underscores, like `MY_API_KEY`.
8686
</Callout>
8787

88+
Inside Function and Custom Tool code, `{{KEY}}` always evaluates to a **string** — the value is bound, never parsed as source. Use `Number()`, a `=== 'true'` comparison, or `JSON.parse()` when you need another type. See [secret placeholders in code](/workflows/blocks/function#placeholders-are-always-strings).
89+
8890
### Personal vs. workspace
8991

9092
| Scope | Visible to | Use for |

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

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
*/
44
import { loggerMock } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { compileCodePlaceholders } from '@/lib/execution/code-placeholders'
7+
import { CodeLanguage } from '@/lib/execution/languages'
68
import { projectResolvedModelInput } from '@/lib/execution/model-input-provenance'
79
import {
810
LARGE_ARRAY_MANIFEST_VERSION,
@@ -125,8 +127,55 @@ function createResolver(
125127
}
126128
}
127129

130+
/** Runs one condition expression through the resolver and returns the value the handler receives. */
131+
async function resolveConditionExpression(
132+
value: string,
133+
environmentVariables: Record<string, string>
134+
): Promise<string> {
135+
const { ctx, resolver } = createResolver()
136+
ctx.environmentVariables = environmentVariables
137+
const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION)
138+
const result = await resolver.resolveInputs(
139+
ctx,
140+
conditionBlock.id,
141+
{ conditions: JSON.stringify([{ id: 'condition-1', title: 'if', value }]) },
142+
conditionBlock
143+
)
144+
return (result.conditions as Array<{ value: string }>)[0].value
145+
}
146+
147+
/**
148+
* Completes the round trip a condition actually takes: resolver, then the execution-boundary
149+
* compiler, then evaluation of the same `Boolean(...)` wrapper `condition-handler.ts` builds.
150+
*/
151+
async function evaluateResolvedCondition(
152+
value: string,
153+
environmentVariables: Record<string, string>
154+
): Promise<boolean> {
155+
const expression = await resolveConditionExpression(value, environmentVariables)
156+
const compiled = await compileCodePlaceholders({
157+
code: `const context = {};\nreturn Boolean(${expression})`,
158+
language: CodeLanguage.JavaScript,
159+
environmentVariables,
160+
})
161+
const installed: string[] = []
162+
try {
163+
for (const binding of compiled.bindings) {
164+
Object.defineProperty(globalThis, binding.name, {
165+
configurable: true,
166+
value: binding.value,
167+
writable: true,
168+
})
169+
installed.push(binding.name)
170+
}
171+
return Boolean(new Function(compiled.code)())
172+
} finally {
173+
for (const name of installed) Reflect.deleteProperty(globalThis, name)
174+
}
175+
}
176+
128177
describe('VariableResolver function block inputs', () => {
129-
it('preserves legacy condition environment substitution semantics', async () => {
178+
it('inlines only structurally inert condition literals and defers the rest to the compiler', async () => {
130179
const { ctx, resolver } = createResolver()
131180
ctx.environmentVariables = {
132181
API_KEY: 'token',
@@ -152,10 +201,76 @@ describe('VariableResolver function block inputs', () => {
152201
expect(result.conditions).toEqual([
153202
{ id: 'condition-1', title: 'if', value: '123 === 123' },
154203
{ id: 'condition-2', title: 'else if', value: 'true === true' },
155-
{ id: 'condition-3', title: 'else if', value: '"Bearer token" === "Bearer token"' },
204+
{
205+
id: 'condition-3',
206+
title: 'else if',
207+
value: '"Bearer {{API_KEY}}" === "Bearer token"',
208+
},
156209
])
157210
})
158211

212+
it('preserves legacy condition outcomes end to end through the boundary compiler', async () => {
213+
const environmentVariables = {
214+
API_KEY: 'token',
215+
BOOLEAN_VALUE: 'true',
216+
NUMBER_VALUE: '123',
217+
NULL_VALUE: 'null',
218+
NEGATIVE: '-5',
219+
EXPONENT: '1e3',
220+
}
221+
const cases = [
222+
{ value: '{{NUMBER_VALUE}} === 123', expected: true },
223+
{ value: '{{BOOLEAN_VALUE}} === true', expected: true },
224+
{ value: '"Bearer {{API_KEY}}" === "Bearer token"', expected: true },
225+
{ value: `'{{API_KEY}}' === 'token'`, expected: true },
226+
{ value: '{{NULL_VALUE}} === null', expected: true },
227+
{ value: '{{NEGATIVE}} === -5', expected: true },
228+
{ value: '{{EXPONENT}} === 1000', expected: true },
229+
{ value: '{{NUMBER_VALUE}} === 999', expected: false },
230+
]
231+
232+
/** A padded value must stay byte-identical: numeric bare, exact string when quoted. */
233+
expect(await evaluateResolvedCondition('{{PADDED}} === 123', { PADDED: ' 123 ' })).toBe(true)
234+
expect(await evaluateResolvedCondition(`'{{PADDED}}' === ' 123 '`, { PADDED: ' 123 ' })).toBe(
235+
true
236+
)
237+
238+
for (const { value, expected } of cases) {
239+
expect(
240+
await evaluateResolvedCondition(value, environmentVariables),
241+
`condition ${value} should evaluate to ${expected}`
242+
).toBe(expected)
243+
}
244+
})
245+
246+
it('stops a secret value from breaking or forging a condition', async () => {
247+
await expect(
248+
evaluateResolvedCondition(`'{{NAME}}' === 'bob'`, { NAME: `x' || true || '` })
249+
).resolves.toBe(false)
250+
await expect(
251+
evaluateResolvedCondition(`'{{NAME}}' === "O'Brien"`, { NAME: "O'Brien" })
252+
).resolves.toBe(true)
253+
await expect(
254+
evaluateResolvedCondition(`'{{NAME}}' === 'a\\nb'`, { NAME: 'a\nb' })
255+
).resolves.toBe(true)
256+
})
257+
258+
it('compares a bare string placeholder instead of throwing a reference error', async () => {
259+
await expect(
260+
evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'alice' })
261+
).resolves.toBe(true)
262+
await expect(evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'bob' })).resolves.toBe(
263+
false
264+
)
265+
})
266+
267+
it('keeps a resolved secret out of the code sent to the execution boundary', async () => {
268+
const resolved = await resolveConditionExpression(`'{{API_KEY}}' === 'token'`, {
269+
API_KEY: 'token',
270+
})
271+
expect(resolved).toBe(`'{{API_KEY}}' === 'token'`)
272+
})
273+
159274
it('does not log malformed Condition source while falling back to legacy resolution', async () => {
160275
const { ctx, resolver } = createResolver()
161276
const secret = 'condition-fallback-secret-value'

apps/sim/executor/variables/resolver.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,34 @@ async function replaceEnvVarsAsync(
109109
return result + template.slice(cursor)
110110
}
111111

112+
/**
113+
* A number, boolean, or null literal, optionally padded with spaces or tabs.
114+
*
115+
* Every character this admits — digits, `.`, `-`, `+`, `e`, the three keywords, spaces, and
116+
* tabs — is inert in both places a Condition placeholder can land. In expression position none
117+
* of them introduces an operator or a comment; inside a string literal none of them terminates
118+
* it. Padding is admitted rather than trimmed so the inlined text stays byte-identical to the
119+
* stored value: whitespace is meaningless in expression position but significant inside a
120+
* quoted string, and only the untrimmed value is correct in both. Line terminators stay out —
121+
* a raw newline would break a single-quoted string.
122+
*/
123+
const STRUCTURALLY_INERT_CONDITION_LITERAL =
124+
/^[ \t]*(?:-?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?|true|false|null)[ \t]*$/
125+
126+
/**
127+
* Whether an environment variable value may be inlined into a Condition expression as source.
128+
*
129+
* Condition expressions are user-authored JavaScript, so an inlined value is parsed as code.
130+
* Only self-contained literals are safe to inline; every other value keeps its `{{NAME}}`
131+
* placeholder and is bound as a string by the execution-boundary compiler instead. That keeps
132+
* `{{COUNT}} === 3` and `{{ENABLED}} === true` comparing as literals — the long-standing
133+
* behavior — while a value containing a quote, newline, or operator can no longer break the
134+
* expression or forge its result.
135+
*/
136+
function isStructurallyInertConditionLiteral(value: string): boolean {
137+
return STRUCTURALLY_INERT_CONDITION_LITERAL.test(value)
138+
}
139+
112140
type ShellQuoteContext = 'single' | 'double' | null
113141
type CodeStringQuoteContext = ShellQuoteContext | 'triple-single' | 'triple-double' | 'template'
114142
type CodeScanMode =
@@ -1419,7 +1447,8 @@ export class VariableResolver {
14191447

14201448
result = await replaceEnvVarsAsync(result, async (match) => {
14211449
const resolved = await this.resolveReference(match, resolutionContext)
1422-
return typeof resolved === 'string' ? resolved : match
1450+
if (typeof resolved !== 'string') return match
1451+
return isStructurallyInertConditionLiteral(resolved) ? resolved : match
14231452
})
14241453
ctx.resolvedSecretTraceRegistry?.recordResolvedInputProjection(
14251454
inputPath,

0 commit comments

Comments
 (0)