Skip to content

fix(executor): stop resolved data from breaking out of generated condition and function code - #7300

Merged
icecrasher321 merged 13 commits into
stagingfrom
fix/condition-expression-injection
Aug 31, 2026
Merged

fix(executor): stop resolved data from breaking out of generated condition and function code#7300
icecrasher321 merged 13 commits into
stagingfrom
fix/condition-expression-injection

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Problem

A Condition expression is compiled by inlining each resolved reference into the author's JavaScript as source text. The literal that gets emitted only ever anticipated the quoting it chose itself — it escapes \, ', and line terminators, then wraps in '…' — but the author's quoting decides which context the value actually lands in.

So trigger data (webhook body, chat message, form field) could close the author's string and execute:

author  : "<start.input>".includes('urgent')
payload : " + (globalThis.__pwned = Object.keys(environmentVariables).join(",")) + "
resolved: "'" + (globalThis.__pwned = Object.keys(environmentVariables).join(",")) + "'".includes('urgent')
verdict : {"matchedIndex":0}        ← branch still matched, run continued
__pwned : OPENAI_API_KEY,DB_URL     ← injected code ran

That runs in the condition sandbox, which is handed the run's whole decrypted environment ({...personalDecrypted, ...workspaceDecrypted} from execution-core.ts) as the environmentVariables global, inside an async wrapper where await fetch(...) is available. The branch verdict is unaffected, so nothing in the run log shows it happened.

Four shapes reached it — double-quoted, template literal, regex literal, and a crafted key on a quoted object reference. The dangerous ones are the shapes that also behave correctly with benign data, so they survive in production: "<x>".includes(…), "<x>".toLowerCase().includes(…), `<x>`.length > n, /<x>/.test(…). ("<x>" === "admin" injects too, but is always false even benignly, so an author would have abandoned it.) A bare <x> === 'admin' was never vulnerable, and '<x>' === 'admin' is a syntax error today, so it never ships.

Same root cause, second site: Function blocks bind block outputs as runtime context variables (safe), but inlined the remaining resolved values as literals. A Variables block can assign trigger data into a workflow variable at runtime, and WorkflowResolver reads the live map — so const x = '<variable.userinput>' spliced attacker text into code. Loop items reached the same path, in JavaScript, Python, and shell.

Fix

Conditions — one shared literal formatter, no behavior change. escapeInertStringContent escapes every terminator of every JavaScript string context, not just the one the emitted literal opens. \", \`, \$, and \/ are identity escapes, so a condition compares byte-for-byte what it compared before; only its ability to parse as anything but data changes. Both condition formatters use it — resolveTemplateWithoutConditionFormatting (the live path) and stringifyForCondition (the fallback, which had the mirrored blind spot: wraps in ", escaped only ").

Deliberately not the quote-context rewrite (getCodeStringQuoteContext + quote + JSON.stringify(v) + quote) that the code-block path uses: it would make "<x>" === "admin" start matching where today it is always false. That is the correct long-term semantics, but it re-routes live workflows and belongs in its own announced change, not a security patch.

A quoted object reference additionally escapes its JSON, since JSON's own structural quotes close the author's string. In expression position the JSON stays a literal (unchanged). In a quoted position the previous output was either a syntax error or an injection, so the escaped form is the only reading that was not already broken.

Function blocks — bind instead of splice. Resolved values now bind as context variables like block outputs always have. Same runtime value, never source. Two exceptions stay inline: numbers/booleans/null (digits and keywords cannot terminate a literal), and a string that names an environment variable and carries no quote — that shape has to stay in source because the placeholder, never the secret, is what is inlined, and the execution-boundary compiler binds it downstream (this is how <variable.indirectSecret> reaches its value).

Defence in depth. Condition evaluation stops shipping the full secret map to the sandbox: it mounts only the names its script references (secretScope: 'selected'). A script that reads the environmentVariables global directly keeps the full map, so nothing that works today breaks.

Tests

  • resolver.test.ts — 5 quotings × 4 payloads must not execute; a crafted-key object must not close the string it is quoted inside; bare and quoted references must still compare what they compared before (including values carrying ", $, /). Both injection tests were confirmed to fail without their respective fix.
  • condition-handler.test.ts — mounts only named secrets, denies all when none are named, keeps all for direct environmentVariables access.
  • block.test.ts — the fallback formatter escapes the quotes it does not open.
  • One existing assertion updated: the navigated manifest element now binds rather than splices (contextVariables holds the element, never the manifest or its array).

bun run test (apps/sim), bun run type-check, biome check, and bun run check:api-validation all pass.

Blast radius

Bare references (the documented form, <agent.score> > 75) are byte-identical. Quoted references keep the same runtime value. Function-block generated code changes shape for spliced scalars/strings — values and display source are unchanged. Quoted object references in conditions change from "syntax error / injectable" to "well-formed string compare".

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 31, 2026 10:59pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents resolved workflow data from becoming executable source in generated condition and function code while narrowing condition sandboxes to the secrets explicitly required.

  • Escapes values inserted into JavaScript condition literal contexts.
  • Binds dynamic function-block values through runtime context variables instead of source splicing.
  • Tracks JavaScript lexical context for regex literals, control-flow heads, comments, and postfix updates.
  • Selects condition secret scope from the author-written expression rather than resolved payload data.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/executor/variables/resolver.ts Resolves dynamic function values through runtime bindings, hardens condition formatting, records authored environment-map access, and improves lexical-context tracking.
apps/sim/executor/handlers/condition/condition-handler.ts Narrows condition sandbox secrets using pre-resolution environment-read metadata and explicitly named placeholders.
apps/sim/executor/utils/code-formatting.ts Adds shared JavaScript string-content escaping for values inserted into condition expressions.
apps/sim/executor/variables/resolvers/block.ts Uses the shared inert string formatter in the fallback condition-value formatting path.
apps/sim/executor/variables/resolver.test.ts Adds regression coverage for injection payloads, object formatting, lexical-context detection, runtime bindings, and legacy condition outcomes.
apps/sim/executor/handlers/condition/condition-handler.test.ts Verifies selected, empty, and full secret scopes and prevents resolved payload data from influencing mounted secrets.
apps/sim/executor/variables/resolvers/block.test.ts Covers fallback formatter behavior across JavaScript quote terminators.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Author-written condition or function code] --> B[Resolve workflow references]
  B --> C{Resolved value may remain an inert literal?}
  C -->|Yes| D[Emit safe literal]
  C -->|No| E[Bind runtime context variable]
  D --> F[Generated sandbox code]
  E --> F
  A --> G[Detect authored secret references]
  G --> H[Select sandbox secret scope]
  H --> F
Loading

Reviews (13): Last reviewed commit: "fix(executor): start a new identifier at..." | Re-trigger Greptile

Comment thread apps/sim/executor/variables/resolver.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts Outdated
Comment thread apps/sim/executor/handlers/condition/condition-handler.ts Outdated
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/variables/resolver.ts
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/handlers/condition/condition-handler.ts Outdated
Comment thread apps/sim/executor/variables/resolver.ts
Comment thread apps/sim/executor/variables/resolver.ts
icecrasher321 and others added 5 commits August 31, 2026 13:59
A Condition expression is compiled by inlining each resolved reference as
source text, and the literal that gets emitted only ever anticipated the
quoting it chose itself. The author's quoting decides the real context, so
`"<start.input>".includes('urgent')` — a shape that works correctly with
benign data — let webhook, chat, or form data close the author's string and
run as JavaScript in the condition sandbox, which receives the workspace's
whole decrypted environment as the `environmentVariables` global. Template
literals, regex literals, and a crafted object key reached the same place.

Both condition formatters now escape every terminator of every JavaScript
string context rather than the one they open. `\"`, `` \` ``, `\$` and `\/`
are identity escapes, so a condition compares exactly what it compared
before; only its ability to parse as anything but data changes. A quoted
object reference additionally escapes its JSON, the only reading of that
shape that was not already a syntax error.

Function blocks bind block outputs as context variables but inlined the
remaining resolved values as literals, so a workflow variable a Variables
block had assigned from trigger data, or a loop item, could close the
string it landed in. Those bind now too. Numbers, booleans, null, and
strings that name an environment variable stay inline: the first three
cannot terminate a literal, and the last has to stay in source because the
placeholder — never the secret — is what is inlined, and the
execution-boundary compiler binds it downstream.

Condition evaluation also stops shipping the full secret map to the
sandbox: it mounts only the names its script references, so a future defect
in this path reaches nothing the condition did not already name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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>
A regex body is the one place a lone quote is not a string delimiter, and
the scanner did not track regex literals at all: `/['"]/` left it believing
everything after it sat inside a string. Every later reference was then
formatted for a context it was not in — a quoted object reference stayed
raw source, and after the previous commit a bare one was emitted as escaped
JSON, which cannot parse, so a valid condition threw instead of routing.

The scan now enters regex mode where a `/` can only be a regex — division
always follows a value, so the preceding token decides — and tracks escapes
and character classes until the closing delimiter. A reference inside a
regex reports its own context, so its JSON is escaped as pattern text
rather than spliced with delimiters the data could forge.

The same scanner decides how function-block references are spliced, so this
also repairs quoting for code that matches on a quote-bearing pattern.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
@icecrasher321
icecrasher321 force-pushed the fix/condition-expression-injection branch from aa5c8f0 to 47da972 Compare August 31, 2026 20:59
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/variables/resolver.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/handlers/condition/condition-handler.ts Outdated
Comment thread apps/sim/executor/variables/resolver.ts Outdated
Comment thread apps/sim/executor/variables/resolver.ts
Comment thread apps/sim/executor/variables/resolver.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts Outdated
Comment thread apps/sim/executor/variables/resolver.ts Outdated
Comment thread apps/sim/executor/variables/resolver.ts
Treating a comment end as the answer was too blunt: `/* c */ if (x)` is a
control-flow head, and calling it a method left a statement-position regex
scanned as division — the failure the comment guard was added to prevent,
moved one shape over. The scan now steps back over comments to the token
that precedes them, so the dot in `p./* c */catch(fn)` is still found and
a keyword after a comment is still a head.

A condition's environment read is placed the same way references are: an
occurrence inside a string, a template, or a regex is text and mounts
nothing, while any executable read — whatever its shape — keeps the map.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/variables/resolver.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Confidence score: 3/5

  • apps/sim/executor/variables/resolver.ts in opensControlFlowHead can misclassify .catch when a block comment between the dot and catch contains another /*, because lastIndexOf selects the embedded delimiter; use the comment’s actual opener and add a nested-delimiter regression test.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/executor/variables/resolver.ts">

<violation number="1" location="apps/sim/executor/variables/resolver.ts:1233">
P1: When a block comment between a property-access dot and `catch` contains another `/*`, `lastIndexOf` selects the embedded delimiter instead of the comment's actual opener. `opensControlFlowHead` then misclassifies `.catch(...)` as a control-flow head, so the following division slash can be scanned as a regex and corrupt later reference formatting. Track the actual non-nesting block-comment boundary when scanning backward.</violation>
</file>

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts Outdated
…k back

Reading backwards cannot tell which characters were code: a block comment
opens at its first delimiter, so `p./* a /* b */catch(fn)` defeated a
search for the nearest `/*` and the call read as a control-flow head again.

The scan already knows — it stepped over that comment on the way in — so
the two facts the check needs, the token before the parenthesis and whether
it followed a property access, are now recorded as it passes and read from
there. No search back through the source, and nothing left for a comment
body to imitate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/variables/resolver.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Confidence score: 3/5

  • apps/sim/executor/variables/resolver.ts misclassifies the division slash after valid postfix ++ or -- as a regex opener, causing later references to be emitted as regex text and direct environment-map reads to be mishandled; update the token classification to distinguish division from regex starts.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/executor/variables/resolver.ts">

<violation number="1" location="apps/sim/executor/variables/resolver.ts:193">
P2: When valid JavaScript uses postfix `++` or `--` before division, this character-based set classifies the division slash as a regex opener. Later references are emitted as regex text and direct environment-map reads can be missed; track postfix operators as value tokens before deciding whether `/` starts a regex.</violation>
</file>

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts
`+` and `-` precede a regex as operators, but doubled they end a value, so
`i++ / 2` was scanning a regex from the division and swallowing whatever
quotes followed it on that line. The check now reads the pair rather than
the single character; a lone `+` still admits `params.n + /re/.test(x)`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/variables/resolver.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts
Comment thread apps/sim/executor/variables/resolver.ts
Comment thread apps/sim/executor/variables/resolver.ts
A token continues only when the character immediately before it belongs to
the same token. Asking the previous *significant* character instead made a
name after a line break look like a continuation, so it kept whatever
property-access answer the last token had: `const seen = params.a.b` on one
line left the `if` on the next carrying `b`'s, which turned the statement
head into a method call and the regex after it into division.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/executor/variables/resolver.ts
Comment thread apps/sim/executor/variables/resolver.ts
@icecrasher321
icecrasher321 merged commit 8e9aeb9 into staging Aug 31, 2026
27 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/condition-expression-injection branch September 1, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant