From d2fbd202cc5eeb9809c21e5fa7d9dbd545d7ca01 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Fri, 14 Aug 2026 13:23:39 -0700 Subject: [PATCH] fix(condition): stop a secret value from breaking or forging a condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../docs/en/workflows/blocks/condition.mdx | 10 ++ .../docs/en/workflows/blocks/function.mdx | 19 +++ .../content/docs/en/workflows/variables.mdx | 2 + apps/sim/executor/variables/resolver.test.ts | 119 +++++++++++++++++- apps/sim/executor/variables/resolver.ts | 31 ++++- 5 files changed, 178 insertions(+), 3 deletions(-) diff --git a/apps/docs/content/docs/en/workflows/blocks/condition.mdx b/apps/docs/content/docs/en/workflows/blocks/condition.mdx index 0dcea7ca603..56a86639b53 100644 --- a/apps/docs/content/docs/en/workflows/blocks/condition.mdx +++ b/apps/docs/content/docs/en/workflows/blocks/condition.mdx @@ -31,6 +31,16 @@ Reference an earlier output inside an expression with a [connection tag](/workfl .endsWith('@company.com') && === 'pro' ``` +Read an [environment variable](/workflows/variables#environment-variables) with `{{KEY}}`: + +```javascript +{{MAX_RETRIES}} === 3 +{{FEATURE_ON}} === true +{{TIER}} === 'pro' +``` + +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. + 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. diff --git a/apps/docs/content/docs/en/workflows/blocks/function.mdx b/apps/docs/content/docs/en/workflows/blocks/function.mdx index 8950ce8e756..e42ebb9eb4e 100644 --- a/apps/docs/content/docs/en/workflows/blocks/function.mdx +++ b/apps/docs/content/docs/en/workflows/blocks/function.mdx @@ -58,6 +58,25 @@ const apiKey = {{API_KEY}}; 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. +### Placeholders are always strings + +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: + +```javascript +const retries = Number({{MAX_RETRIES}}); +const enabled = {{FEATURE_ON}} === 'true'; +const patterns = JSON.parse({{PATTERN_LIST}}); +``` + +In Python, use `int()`, `== "true"`, and `json.loads()` the same way. + +Two cases are easy to miss: + +- **Booleans.** A bare `if ({{FEATURE_ON}})` is always true, because the string `"false"` is truthy. Compare against `'true'` instead. +- **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. + +[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. + JavaScript regex literals can contain a placeholder: ```javascript diff --git a/apps/docs/content/docs/en/workflows/variables.mdx b/apps/docs/content/docs/en/workflows/variables.mdx index 1ea2e7f4ed4..2045c84e277 100644 --- a/apps/docs/content/docs/en/workflows/variables.mdx +++ b/apps/docs/content/docs/en/workflows/variables.mdx @@ -85,6 +85,8 @@ Reference them with double curly braces in any block field, including Agent syst Environment variable names must start with a letter or underscore and contain only letters, numbers, and underscores, like `MY_API_KEY`. +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). + ### Personal vs. workspace | Scope | Visible to | Use for | diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 1bc54b1c0e7..aa0c8c93b42 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -3,6 +3,8 @@ */ import { loggerMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { compileCodePlaceholders } from '@/lib/execution/code-placeholders' +import { CodeLanguage } from '@/lib/execution/languages' import { projectResolvedModelInput } from '@/lib/execution/model-input-provenance' import { LARGE_ARRAY_MANIFEST_VERSION, @@ -125,8 +127,55 @@ function createResolver( } } +/** Runs one condition expression through the resolver and returns the value the handler receives. */ +async function resolveConditionExpression( + value: string, + environmentVariables: Record +): Promise { + const { ctx, resolver } = createResolver() + ctx.environmentVariables = environmentVariables + const conditionBlock = createBlock('condition', 'Condition', BlockType.CONDITION) + const result = await resolver.resolveInputs( + ctx, + conditionBlock.id, + { conditions: JSON.stringify([{ id: 'condition-1', title: 'if', value }]) }, + conditionBlock + ) + return (result.conditions as Array<{ value: string }>)[0].value +} + +/** + * Completes the round trip a condition actually takes: resolver, then the execution-boundary + * compiler, then evaluation of the same `Boolean(...)` wrapper `condition-handler.ts` builds. + */ +async function evaluateResolvedCondition( + value: string, + environmentVariables: Record +): Promise { + const expression = await resolveConditionExpression(value, environmentVariables) + const compiled = await compileCodePlaceholders({ + code: `const context = {};\nreturn Boolean(${expression})`, + language: CodeLanguage.JavaScript, + environmentVariables, + }) + const installed: string[] = [] + try { + for (const binding of compiled.bindings) { + Object.defineProperty(globalThis, binding.name, { + configurable: true, + value: binding.value, + writable: true, + }) + installed.push(binding.name) + } + return Boolean(new Function(compiled.code)()) + } finally { + for (const name of installed) Reflect.deleteProperty(globalThis, name) + } +} + describe('VariableResolver function block inputs', () => { - it('preserves legacy condition environment substitution semantics', async () => { + it('inlines only structurally inert condition literals and defers the rest to the compiler', async () => { const { ctx, resolver } = createResolver() ctx.environmentVariables = { API_KEY: 'token', @@ -152,10 +201,76 @@ describe('VariableResolver function block inputs', () => { expect(result.conditions).toEqual([ { id: 'condition-1', title: 'if', value: '123 === 123' }, { id: 'condition-2', title: 'else if', value: 'true === true' }, - { id: 'condition-3', title: 'else if', value: '"Bearer token" === "Bearer token"' }, + { + id: 'condition-3', + title: 'else if', + value: '"Bearer {{API_KEY}}" === "Bearer token"', + }, ]) }) + it('preserves legacy condition outcomes end to end through the boundary compiler', async () => { + const environmentVariables = { + API_KEY: 'token', + BOOLEAN_VALUE: 'true', + NUMBER_VALUE: '123', + NULL_VALUE: 'null', + NEGATIVE: '-5', + EXPONENT: '1e3', + } + const cases = [ + { value: '{{NUMBER_VALUE}} === 123', expected: true }, + { value: '{{BOOLEAN_VALUE}} === true', expected: true }, + { value: '"Bearer {{API_KEY}}" === "Bearer token"', expected: true }, + { value: `'{{API_KEY}}' === 'token'`, expected: true }, + { value: '{{NULL_VALUE}} === null', expected: true }, + { value: '{{NEGATIVE}} === -5', expected: true }, + { value: '{{EXPONENT}} === 1000', expected: true }, + { value: '{{NUMBER_VALUE}} === 999', expected: false }, + ] + + /** A padded value must stay byte-identical: numeric bare, exact string when quoted. */ + expect(await evaluateResolvedCondition('{{PADDED}} === 123', { PADDED: ' 123 ' })).toBe(true) + expect(await evaluateResolvedCondition(`'{{PADDED}}' === ' 123 '`, { PADDED: ' 123 ' })).toBe( + true + ) + + for (const { value, expected } of cases) { + expect( + await evaluateResolvedCondition(value, environmentVariables), + `condition ${value} should evaluate to ${expected}` + ).toBe(expected) + } + }) + + it('stops a secret value from breaking or forging a condition', async () => { + await expect( + evaluateResolvedCondition(`'{{NAME}}' === 'bob'`, { NAME: `x' || true || '` }) + ).resolves.toBe(false) + await expect( + evaluateResolvedCondition(`'{{NAME}}' === "O'Brien"`, { NAME: "O'Brien" }) + ).resolves.toBe(true) + await expect( + evaluateResolvedCondition(`'{{NAME}}' === 'a\\nb'`, { NAME: 'a\nb' }) + ).resolves.toBe(true) + }) + + it('compares a bare string placeholder instead of throwing a reference error', async () => { + await expect( + evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'alice' }) + ).resolves.toBe(true) + await expect(evaluateResolvedCondition(`{{NAME}} === 'alice'`, { NAME: 'bob' })).resolves.toBe( + false + ) + }) + + it('keeps a resolved secret out of the code sent to the execution boundary', async () => { + const resolved = await resolveConditionExpression(`'{{API_KEY}}' === 'token'`, { + API_KEY: 'token', + }) + expect(resolved).toBe(`'{{API_KEY}}' === 'token'`) + }) + it('does not log malformed Condition source while falling back to legacy resolution', async () => { const { ctx, resolver } = createResolver() const secret = 'condition-fallback-secret-value' diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index b2cbd009555..f5e50e2d50a 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -109,6 +109,34 @@ async function replaceEnvVarsAsync( return result + template.slice(cursor) } +/** + * A number, boolean, or null literal, optionally padded with spaces or tabs. + * + * Every character this admits — digits, `.`, `-`, `+`, `e`, the three keywords, spaces, and + * tabs — is inert in both places a Condition placeholder can land. In expression position none + * of them introduces an operator or a comment; inside a string literal none of them terminates + * it. Padding is admitted rather than trimmed so the inlined text stays byte-identical to the + * stored value: whitespace is meaningless in expression position but significant inside a + * quoted string, and only the untrimmed value is correct in both. Line terminators stay out — + * a raw newline would break a single-quoted string. + */ +const STRUCTURALLY_INERT_CONDITION_LITERAL = + /^[ \t]*(?:-?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?|true|false|null)[ \t]*$/ + +/** + * Whether an environment variable value may be inlined into a Condition expression as source. + * + * Condition expressions are user-authored JavaScript, so an inlined value is parsed as code. + * Only self-contained literals are safe to inline; every other value keeps its `{{NAME}}` + * placeholder and is bound as a string by the execution-boundary compiler instead. That keeps + * `{{COUNT}} === 3` and `{{ENABLED}} === true` comparing as literals — the long-standing + * behavior — while a value containing a quote, newline, or operator can no longer break the + * expression or forge its result. + */ +function isStructurallyInertConditionLiteral(value: string): boolean { + return STRUCTURALLY_INERT_CONDITION_LITERAL.test(value) +} + type ShellQuoteContext = 'single' | 'double' | null type CodeStringQuoteContext = ShellQuoteContext | 'triple-single' | 'triple-double' | 'template' type CodeScanMode = @@ -1419,7 +1447,8 @@ export class VariableResolver { result = await replaceEnvVarsAsync(result, async (match) => { const resolved = await this.resolveReference(match, resolutionContext) - return typeof resolved === 'string' ? resolved : match + if (typeof resolved !== 'string') return match + return isStructurallyInertConditionLiteral(resolved) ? resolved : match }) ctx.resolvedSecretTraceRegistry?.recordResolvedInputProjection( inputPath,