Skip to content

Commit 954012c

Browse files
authored
fix(execution): bound reference and context processing (#8344)
* fix(execution): bound reference and context processing * fix(execution): preserve compatibility and tighten input budgets
1 parent c2cce49 commit 954012c

11 files changed

Lines changed: 353 additions & 89 deletions

File tree

‎apps/sim/lib/api/contracts/hotspots.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types'
1010
import { DEFAULT_CODE_LANGUAGE } from '@/lib/execution/languages'
1111
import { PRIVATE_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata'
1212
import { MAX_BLOCK_MOUNTED_FILES } from '@/lib/execution/remote-sandbox/sandbox-paths'
13+
import { MAX_FUNCTION_CODE_LENGTH } from '@/lib/function-execution/limits'
1314
import {
1415
MAX_PII_VALIDATION_DETECTED_ENTITIES,
1516
MAX_PII_VALIDATION_TEXT_CHARACTERS,
@@ -165,8 +166,8 @@ const functionOutputFileSchema = z
165166

166167
export const functionExecuteBodySchema = z
167168
.object({
168-
code: z.string().min(1, 'Code is required'),
169-
sourceCode: z.string().optional(),
169+
code: z.string().min(1, 'Code is required').max(MAX_FUNCTION_CODE_LENGTH),
170+
sourceCode: z.string().max(MAX_FUNCTION_CODE_LENGTH).optional(),
170171
params: unknownRecordSchema.optional().default({}),
171172
timeout: z.coerce.number().int().positive().optional(),
172173
language: z.string().optional().default(DEFAULT_CODE_LANGUAGE),

‎apps/sim/lib/api/contracts/mothership-chats.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import {
99
} from '@/lib/api/contracts/secret-mount-policy'
1010
import { defineRouteContract } from '@/lib/api/contracts/types'
1111
import type { RESOLVED_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata'
12+
import {
13+
MAX_CHAT_CONTEXT_LABEL_LENGTH,
14+
MAX_CHAT_CONTEXTS,
15+
MAX_CHAT_MESSAGE_LENGTH,
16+
} from '@/lib/mothership/chat/context-limits'
1217
import { ChatPayloadSchema } from '@/lib/mothership/generated/protocol'
1318
import type { AgentStreamEvent, TextDeltaClassification } from '@/providers/stream-events'
1419

@@ -71,7 +76,11 @@ export const markMothershipChatReadContract = defineRouteContract({
7176

7277
const mothershipExecuteMessageSchema = z.object({
7378
role: z.enum(['system', 'user', 'assistant']),
74-
content: z.string(),
79+
content: z.string().max(MAX_CHAT_MESSAGE_LENGTH),
80+
})
81+
82+
const mothershipContextInputSchema = scheduleContextSchema.extend({
83+
label: z.string().max(MAX_CHAT_CONTEXT_LABEL_LENGTH),
7584
})
7685

7786
const mothershipExecuteFileAttachmentSchema = z
@@ -134,7 +143,7 @@ export const mothershipExecuteBodySchema = z.object({
134143
* mirroring the interactive chat path. Headless executions use this to pass
135144
* captured contexts into the run without a live client.
136145
*/
137-
contexts: z.array(scheduleContextSchema).optional(),
146+
contexts: z.array(mothershipContextInputSchema).max(MAX_CHAT_CONTEXTS).optional(),
138147
mcpTools: z.array(mothershipExecuteMcpToolSchema).optional(),
139148
workflowId: z.string().optional(),
140149
executionId: z.string().optional(),
@@ -162,7 +171,8 @@ export const mothershipChatGetQuerySchema = z
162171

163172
export const mothershipChatPostEnvelopeSchema = z
164173
.object({
165-
message: z.string().optional(),
174+
message: z.string().max(MAX_CHAT_MESSAGE_LENGTH).optional(),
175+
contexts: z.array(mothershipContextInputSchema).max(MAX_CHAT_CONTEXTS).optional(),
166176
chatId: z.string().optional(),
167177
workflowId: z.string().optional(),
168178
workspaceId: z.string().optional(),

‎apps/sim/lib/function-execution/execute-request.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3157,6 +3157,53 @@ describe('Function execution request', () => {
31573157
})
31583158

31593159
describe('Template Variable Resolution', () => {
3160+
it.each([
3161+
{ code: ' '.repeat(1024 * 1024 + 1), reason: 'source length' },
3162+
{
3163+
code: 'return 1',
3164+
sourceCode: ' '.repeat(1024 * 1024 + 1),
3165+
reason: 'diagnostic source length',
3166+
},
3167+
{ code: `/* ${'< a.value>'.repeat(10_001)} */ return 1`, reason: 'block references' },
3168+
{
3169+
code: `/* ${'<variable.total>'.repeat(10_001)} */ return 1`,
3170+
reason: 'workflow references',
3171+
},
3172+
])('rejects excessive $reason before execution', async ({ code, sourceCode }) => {
3173+
const response = await POST(
3174+
createMockRequest('POST', {
3175+
code,
3176+
sourceCode,
3177+
workflowVariables: { total: { name: 'total', type: 'number', value: 1 } },
3178+
})
3179+
)
3180+
expect(response.status).toBe(400)
3181+
})
3182+
3183+
it('preserves repeated values, legacy variable precedence, and skipped-block references', async () => {
3184+
mockExecuteInIsolatedVM.mockImplementationOnce(async (request) => ({
3185+
result: await runInNewContext(`(async () => { ${request.code} })()`, {
3186+
...request.contextVariables,
3187+
}),
3188+
stdout: '',
3189+
}))
3190+
const response = await POST(
3191+
createMockRequest('POST', {
3192+
code: 'return [<source.result>.total, <source.result>.total, <variable.totalamount>, <variable.totalamount>, typeof < skipped.value>, <variable.tax-rate>, <variable.tax_rate>]',
3193+
blockNameMapping: { source: 'source-id', skipped: 'skipped-id' },
3194+
blockData: { 'source-id': { result: '{"total":3}' } },
3195+
workflowVariables: {
3196+
first: { name: 'Total amount', type: 'number', value: '7' },
3197+
second: { name: 'totalamount', type: 'number', value: '99' },
3198+
taxRate: { name: 'tax-rate', type: 'number', value: '11' },
3199+
legacyTaxRate: { name: 'tax_rate', type: 'number', value: '12' },
3200+
},
3201+
})
3202+
)
3203+
expect(response.status).toBe(200)
3204+
expect((await response.json()).output.result).toEqual([3, 3, 7, 7, 'undefined', 11, 11])
3205+
})
3206+
31603207
it('keeps an exact-name/exact-value JavaScript secret out of source and returns its raw runtime value with private provenance', async () => {
31613208
mockExecuteInIsolatedVM.mockResolvedValueOnce({ result: 'Test', stdout: '' })
31623209

‎apps/sim/lib/function-execution/execute-request.ts‎

Lines changed: 56 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { sha256Hex } from '@sim/security/hash'
88
import { getErrorMessage } from '@sim/utils/errors'
99
import { generateShortId } from '@sim/utils/id'
1010
import { toRecord } from '@sim/utils/object'
11-
import { escapeRegExp } from '@sim/utils/string'
1211
import { NextResponse } from 'next/server'
1312
import type { ParsedFunctionExecuteBody } from '@/lib/api/contracts'
1413
import { isMothershipSandboxEnabled, isRemoteSandboxEnabled } from '@/lib/core/config/env-flags'
@@ -87,6 +86,7 @@ import {
8786
} from '@/lib/execution/remote-sandbox/sandbox-paths'
8887
import type { SandboxCollectedFile, SandboxFile } from '@/lib/execution/remote-sandbox/types'
8988
import { isExecutionResourceLimitError } from '@/lib/execution/resource-errors'
89+
import { MAX_FUNCTION_REFERENCES } from '@/lib/function-execution/limits'
9090
import type { SandboxExportedFile } from '@/lib/function-execution/output'
9191
import { planUserFileMounts, resolveUserFileMounts } from '@/lib/function-execution/sandbox-mounts'
9292
import {
@@ -750,38 +750,33 @@ function scrubInternalIdentifiers(message: string, identifiers: readonly string[
750750

751751
function resolveWorkflowVariables(
752752
code: string,
753-
workflowVariables: Record<string, any>,
754-
contextVariables: Record<string, any>
753+
workflowVariables: Record<string, unknown>,
754+
contextVariables: Record<string, unknown>
755755
): string {
756-
let resolvedCode = code
757-
758-
const regex = createWorkflowVariablePattern()
759-
let match: RegExpExecArray | null
760-
const replacements: Array<{
761-
match: string
762-
index: number
763-
variableName: string
764-
variableValue: unknown
765-
}> = []
766-
767-
while ((match = regex.exec(code)) !== null) {
768-
const variableName = match[1].trim()
769-
770-
const foundVariable = Object.entries(workflowVariables).find(
771-
([_, variable]) => normalizeName(variable.name || '') === variableName
772-
)
773-
774-
if (!foundVariable) {
775-
const availableVars = Object.values(workflowVariables)
776-
.map((v) => v.name)
777-
.filter(Boolean)
756+
const variablesByName = new Map<string, Record<string, unknown>>()
757+
for (const value of Object.values(workflowVariables)) {
758+
const variable = toRecord(value)
759+
if (typeof variable.name !== 'string') continue
760+
const name = normalizeName(variable.name)
761+
if (!variablesByName.has(name)) variablesByName.set(name, variable)
762+
}
763+
const replacements = new Map<string, string>()
764+
const boundNames = new Set<string>()
765+
766+
return code.replace(createWorkflowVariablePattern(), (_match, name: string) => {
767+
const variableName = name.trim()
768+
const cached = replacements.get(variableName)
769+
if (cached !== undefined) return cached
770+
771+
const variable = variablesByName.get(variableName)
772+
if (!variable) {
773+
const availableVars = [...variablesByName.values()].map((value) => value.name).filter(Boolean)
778774
throw new Error(
779775
`Variable "${variableName}" doesn't exist.` +
780776
(availableVars.length > 0 ? ` Available: ${availableVars.join(', ')}` : '')
781777
)
782778
}
783779

784-
const variable = foundVariable[1]
785780
let variableValue: unknown = variable.value
786781

787782
if (variable.value !== undefined && variable.value !== null) {
@@ -805,24 +800,15 @@ function resolveWorkflowVariables(
805800
}
806801
}
807802

808-
replacements.push({
809-
match: match[0],
810-
index: match.index,
811-
variableName,
812-
variableValue,
813-
})
814-
}
815-
816-
for (let i = replacements.length - 1; i >= 0; i--) {
817-
const { match: matchStr, index, variableName, variableValue } = replacements[i]
818-
819803
const safeVarName = `__variable_${variableName.replace(/[^a-zA-Z0-9_]/g, '_')}`
820-
contextVariables[safeVarName] = variableValue
821-
resolvedCode =
822-
resolvedCode.slice(0, index) + safeVarName + resolvedCode.slice(index + matchStr.length)
823-
}
824-
825-
return resolvedCode
804+
// The original reverse rewrite gave the first reference precedence on binding-name collisions.
805+
if (!boundNames.has(safeVarName)) {
806+
contextVariables[safeVarName] = variableValue
807+
boundNames.add(safeVarName)
808+
}
809+
replacements.set(variableName, safeVarName)
810+
return safeVarName
811+
})
826812
}
827813

828814
/**
@@ -869,13 +855,12 @@ function resolveTagVariables(
869855
contextVariables: Record<string, unknown>,
870856
language = 'javascript'
871857
): string {
872-
let resolvedCode = code
873858
const undefinedLiteral = language === 'python' ? 'None' : 'undefined'
859+
const replacements = new Map<string, string | undefined>()
874860

875-
const tagMatches = resolvedCode.match(TAG_PATTERN) || []
876-
877-
for (const match of tagMatches) {
861+
return code.replace(TAG_PATTERN, (match) => {
878862
const tagName = match.slice(REFERENCE.START.length, -REFERENCE.END.length).trim()
863+
if (replacements.has(tagName)) return replacements.get(tagName) ?? match
879864
const pathParts = tagName.split(REFERENCE.PATH_DELIMITER)
880865
const blockName = pathParts[0]
881866
const fieldPath = pathParts.slice(1)
@@ -887,14 +872,15 @@ function resolveTagVariables(
887872
})
888873

889874
if (!result) {
890-
continue
875+
replacements.set(tagName, undefined)
876+
return match
891877
}
892878

893879
let tagValue = result.value
894880

895881
if (tagValue === undefined) {
896-
resolvedCode = resolvedCode.replace(new RegExp(escapeRegExp(match), 'g'), undefinedLiteral)
897-
continue
882+
replacements.set(tagName, undefinedLiteral)
883+
return undefinedLiteral
898884
}
899885

900886
if (typeof tagValue === 'string') {
@@ -910,10 +896,9 @@ function resolveTagVariables(
910896

911897
const safeVarName = `__tag_${tagName.replace(/_/g, '_1').replace(/\./g, '_0')}`
912898
contextVariables[safeVarName] = tagValue
913-
resolvedCode = resolvedCode.replace(new RegExp(escapeRegExp(match), 'g'), safeVarName)
914-
}
915-
916-
return resolvedCode
899+
replacements.set(tagName, safeVarName)
900+
return safeVarName
901+
})
917902
}
918903

919904
/**
@@ -2281,6 +2266,23 @@ export async function executeFunctionRequest(
22812266
)
22822267
includePrivateResolvedSecretNames = privateResolvedSecretNamesMetadataType !== undefined
22832268

2269+
let referenceCount = 0
2270+
for (const _match of body.code.matchAll(TAG_PATTERN)) {
2271+
if (++referenceCount > MAX_FUNCTION_REFERENCES) {
2272+
return appendPrivateResolvedSecretNames(
2273+
NextResponse.json(
2274+
{
2275+
success: false,
2276+
error: `Function code exceeds the maximum of ${MAX_FUNCTION_REFERENCES} references`,
2277+
},
2278+
{ status: 400 }
2279+
),
2280+
includePrivateResolvedSecretNames ? [] : null,
2281+
privateResolvedSecretNamesMetadataType
2282+
)
2283+
}
2284+
}
2285+
22842286
const mountedWorkspaceFileProvenance = inspectMountedWorkspaceFileProvenance(req.headers, body)
22852287
if (mountedWorkspaceFileProvenance.status === 'invalid') {
22862288
return appendPrivateResolvedSecretNames(
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
/** Bounds source scanning and the bindings created before sandbox execution. */
2+
export const MAX_FUNCTION_CODE_LENGTH = 1024 * 1024
3+
export const MAX_FUNCTION_REFERENCES = 10_000

‎apps/sim/lib/mothership/agent-cli/engines.test.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,90 @@ const DEPS_STATE = {
136136
}
137137

138138
describe('workflows deps', () => {
139+
it('stops reading a wide object when its traversal budget is exhausted', async () => {
140+
const value: Record<string, unknown> = {}
141+
for (let index = 0; index < 10_001; index++) {
142+
Object.defineProperty(value, String(index), {
143+
enumerable: true,
144+
get() {
145+
if (index === 10_000) throw new Error('Read beyond the traversal budget')
146+
return ''
147+
},
148+
})
149+
}
150+
const state = {
151+
...DEPS_STATE,
152+
blocks: {
153+
...DEPS_STATE.blocks,
154+
target: { ...DEPS_STATE.blocks.target, subBlocks: { code: { value } } },
155+
},
156+
}
157+
const result = await runEngine(
158+
'workflows deps',
159+
['wf-1', 'target'],
160+
runtimeWith({ [STATE_PATH]: { data: state } }),
161+
{}
162+
)
163+
expect(result.exitCode).toBe(1)
164+
expect(result.stderr).toMatch(/exceeds.*values/i)
165+
expect(result.stdout).toBe('')
166+
})
167+
168+
it.each([
169+
{ reason: 'text size', value: 'x'.repeat(1024 * 1024 + 1) },
170+
{ reason: 'reference count', value: '<fetchrows.result>'.repeat(10_001) },
171+
{ reason: 'nested value count', value: Array.from({ length: 10_001 }, () => '') },
172+
{
173+
reason: 'path depth',
174+
value: `<fetchrows.${Array.from({ length: 129 }, () => 'nested').join('.')}>`,
175+
},
176+
])(
177+
'refuses excessive $reason instead of returning an incomplete dependency report',
178+
async ({ value }) => {
179+
const state = {
180+
...DEPS_STATE,
181+
blocks: {
182+
...DEPS_STATE.blocks,
183+
target: { ...DEPS_STATE.blocks.target, subBlocks: { code: { value } } },
184+
},
185+
}
186+
const result = await runEngine(
187+
'workflows deps',
188+
['wf-1', 'target'],
189+
runtimeWith({ [STATE_PATH]: { data: state } }),
190+
{}
191+
)
192+
expect(result.exitCode).toBe(1)
193+
expect(result.stderr).toMatch(/exceeds|maximum/i)
194+
expect(result.stdout).toBe('')
195+
}
196+
)
197+
198+
it('groups block aliases and duplicate paths without changing first-reference order', async () => {
199+
const state = structuredClone(DEPS_STATE)
200+
state.blocks.target.subBlocks.code.value =
201+
'<missing.value> <fetchrows.result> <fetch.result> <fetchrows.result.id> <fetchrows.result.id> {{TOKEN}} {{TOKEN}}'
202+
const result = await runEngine(
203+
'workflows deps',
204+
['wf-1', 'target'],
205+
runtimeWith({ [STATE_PATH]: { data: state } }),
206+
{}
207+
)
208+
const report = JSON.parse(result.stdout)
209+
expect(report.references).toEqual([
210+
{ token: 'missing.value', kind: 'unknown' },
211+
{
212+
token: 'fetchrows.result',
213+
kind: 'block',
214+
blockId: 'fetch',
215+
blockName: 'Fetch rows',
216+
paths: ['result', 'result.id'],
217+
},
218+
])
219+
expect(report.env).toEqual(['TOKEN'])
220+
expect(report.mock['Fetch rows']).toEqual({ result: { id: null } })
221+
})
222+
139223
it('builds indexed mocks that round-trip through the actual reference navigator', async () => {
140224
const state = structuredClone(DEPS_STATE)
141225
state.blocks.target.subBlocks.code.value =

0 commit comments

Comments
 (0)