Skip to content

Commit b913cff

Browse files
fix(envvars): resolution standardized (#2957)
* fix(envvars): resolution standardized * remove comments * address bugbot * fix highlighting for env vars * remove comments * address greptile * address bugbot
1 parent 428781c commit b913cff

File tree

33 files changed

+387
-725
lines changed

33 files changed

+387
-725
lines changed

apps/sim/app/api/copilot/execute-tool/route.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,11 @@ export async function POST(req: NextRequest) {
104104
})
105105

106106
// Build execution params starting with LLM-provided arguments
107-
// Resolve all {{ENV_VAR}} references in the arguments
107+
// Resolve all {{ENV_VAR}} references in the arguments (deep for nested objects)
108108
const executionParams: Record<string, any> = resolveEnvVarReferences(
109109
toolArgs,
110110
decryptedEnvVars,
111-
{
112-
resolveExactMatch: true,
113-
allowEmbedded: true,
114-
trimKeys: true,
115-
onMissing: 'keep',
116-
deep: true,
117-
}
111+
{ deep: true }
118112
) as Record<string, any>
119113

120114
logger.info(`[${tracker.requestId}] Resolved env var references in arguments`, {

apps/sim/app/api/mcp/servers/test-connection/route.ts

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
3-
import { getEffectiveDecryptedEnv } from '@/lib/environment/utils'
43
import { McpClient } from '@/lib/mcp/client'
54
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
6-
import type { McpServerConfig, McpTransport } from '@/lib/mcp/types'
5+
import { resolveMcpConfigEnvVars } from '@/lib/mcp/resolve-config'
6+
import type { McpTransport } from '@/lib/mcp/types'
77
import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
8-
import { resolveEnvVarReferences } from '@/executor/utils/reference-validation'
98

109
const logger = createLogger('McpServerTestAPI')
1110

@@ -19,30 +18,6 @@ function isUrlBasedTransport(transport: McpTransport): boolean {
1918
return transport === 'streamable-http'
2019
}
2120

22-
/**
23-
* Resolve environment variables in strings
24-
*/
25-
function resolveEnvVars(value: string, envVars: Record<string, string>): string {
26-
const missingVars: string[] = []
27-
const resolvedValue = resolveEnvVarReferences(value, envVars, {
28-
allowEmbedded: true,
29-
resolveExactMatch: true,
30-
trimKeys: true,
31-
onMissing: 'keep',
32-
deep: false,
33-
missingKeys: missingVars,
34-
}) as string
35-
36-
if (missingVars.length > 0) {
37-
const uniqueMissing = Array.from(new Set(missingVars))
38-
uniqueMissing.forEach((envKey) => {
39-
logger.warn(`Environment variable "${envKey}" not found in MCP server test`)
40-
})
41-
}
42-
43-
return resolvedValue
44-
}
45-
4621
interface TestConnectionRequest {
4722
name: string
4823
transport: McpTransport
@@ -96,39 +71,30 @@ export const POST = withMcpAuth('write')(
9671
)
9772
}
9873

99-
let resolvedUrl = body.url
100-
let resolvedHeaders = body.headers || {}
101-
102-
try {
103-
const envVars = await getEffectiveDecryptedEnv(userId, workspaceId)
104-
105-
if (resolvedUrl) {
106-
resolvedUrl = resolveEnvVars(resolvedUrl, envVars)
107-
}
108-
109-
const resolvedHeadersObj: Record<string, string> = {}
110-
for (const [key, value] of Object.entries(resolvedHeaders)) {
111-
resolvedHeadersObj[key] = resolveEnvVars(value, envVars)
112-
}
113-
resolvedHeaders = resolvedHeadersObj
114-
} catch (envError) {
115-
logger.warn(
116-
`[${requestId}] Failed to resolve environment variables, using raw values:`,
117-
envError
118-
)
119-
}
120-
121-
const testConfig: McpServerConfig = {
74+
// Build initial config for resolution
75+
const initialConfig = {
12276
id: `test-${requestId}`,
12377
name: body.name,
12478
transport: body.transport,
125-
url: resolvedUrl,
126-
headers: resolvedHeaders,
79+
url: body.url,
80+
headers: body.headers || {},
12781
timeout: body.timeout || 10000,
12882
retries: 1, // Only one retry for tests
12983
enabled: true,
13084
}
13185

86+
// Resolve env vars using shared utility (non-strict mode for testing)
87+
const { config: testConfig, missingVars } = await resolveMcpConfigEnvVars(
88+
initialConfig,
89+
userId,
90+
workspaceId,
91+
{ strict: false }
92+
)
93+
94+
if (missingVars.length > 0) {
95+
logger.warn(`[${requestId}] Some environment variables not found:`, { missingVars })
96+
}
97+
13298
const testSecurityPolicy = {
13399
requireConsent: false,
134100
auditLevel: 'none' as const,

apps/sim/app/api/webhooks/[id]/route.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { getSession } from '@/lib/auth'
77
import { validateInteger } from '@/lib/core/security/input-validation'
88
import { PlatformEvents } from '@/lib/core/telemetry'
99
import { generateRequestId } from '@/lib/core/utils/request'
10+
import { resolveEnvVarsInObject } from '@/lib/webhooks/env-resolver'
1011
import {
1112
cleanupExternalWebhook,
1213
createExternalWebhookSubscription,
@@ -112,9 +113,9 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise<
112113
}
113114
}
114115

116+
const originalProviderConfig = providerConfig
115117
let resolvedProviderConfig = providerConfig
116118
if (providerConfig) {
117-
const { resolveEnvVarsInObject } = await import('@/lib/webhooks/env-resolver')
118119
const webhookDataForResolve = await db
119120
.select({
120121
workspaceId: workflow.workspaceId,
@@ -230,19 +231,23 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise<
230231
hasFailedCountUpdate: failedCount !== undefined,
231232
})
232233

233-
// Merge providerConfig to preserve credential-related fields
234234
let finalProviderConfig = webhooks[0].webhook.providerConfig
235-
if (providerConfig !== undefined) {
235+
if (providerConfig !== undefined && originalProviderConfig) {
236236
const existingConfig = existingProviderConfig
237237
finalProviderConfig = {
238-
...nextProviderConfig,
238+
...originalProviderConfig,
239239
credentialId: existingConfig.credentialId,
240240
credentialSetId: existingConfig.credentialSetId,
241241
userId: existingConfig.userId,
242242
historyId: existingConfig.historyId,
243243
lastCheckedTimestamp: existingConfig.lastCheckedTimestamp,
244244
setupCompleted: existingConfig.setupCompleted,
245-
externalId: nextProviderConfig.externalId ?? existingConfig.externalId,
245+
externalId: existingConfig.externalId,
246+
}
247+
for (const [key, value] of Object.entries(nextProviderConfig)) {
248+
if (!(key in originalProviderConfig)) {
249+
;(finalProviderConfig as Record<string, unknown>)[key] = value
250+
}
246251
}
247252
}
248253

apps/sim/app/api/webhooks/route.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { type NextRequest, NextResponse } from 'next/server'
77
import { getSession } from '@/lib/auth'
88
import { PlatformEvents } from '@/lib/core/telemetry'
99
import { generateRequestId } from '@/lib/core/utils/request'
10+
import { resolveEnvVarsInObject } from '@/lib/webhooks/env-resolver'
1011
import { createExternalWebhookSubscription } from '@/lib/webhooks/provider-subscriptions'
1112
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1213

@@ -298,14 +299,10 @@ export async function POST(request: NextRequest) {
298299
}
299300
}
300301

301-
let savedWebhook: any = null // Variable to hold the result of save/update
302-
303-
// Use the original provider config - Gmail/Outlook configuration functions will inject userId automatically
304-
const finalProviderConfig = providerConfig || {}
305-
306-
const { resolveEnvVarsInObject } = await import('@/lib/webhooks/env-resolver')
302+
let savedWebhook: any = null
303+
const originalProviderConfig = providerConfig || {}
307304
let resolvedProviderConfig = await resolveEnvVarsInObject(
308-
finalProviderConfig,
305+
originalProviderConfig,
309306
userId,
310307
workflowRecord.workspaceId || undefined
311308
)
@@ -469,6 +466,8 @@ export async function POST(request: NextRequest) {
469466
providerConfig: providerConfigOverride,
470467
})
471468

469+
const configToSave = { ...originalProviderConfig }
470+
472471
try {
473472
const result = await createExternalWebhookSubscription(
474473
request,
@@ -477,7 +476,13 @@ export async function POST(request: NextRequest) {
477476
userId,
478477
requestId
479478
)
480-
resolvedProviderConfig = result.updatedProviderConfig as Record<string, unknown>
479+
const updatedConfig = result.updatedProviderConfig as Record<string, unknown>
480+
for (const [key, value] of Object.entries(updatedConfig)) {
481+
if (!(key in originalProviderConfig)) {
482+
configToSave[key] = value
483+
}
484+
}
485+
resolvedProviderConfig = updatedConfig
481486
externalSubscriptionCreated = result.externalSubscriptionCreated
482487
} catch (err) {
483488
logger.error(`[${requestId}] Error creating external webhook subscription`, err)
@@ -490,25 +495,22 @@ export async function POST(request: NextRequest) {
490495
)
491496
}
492497

493-
// Now save to database (only if subscription succeeded or provider doesn't need external subscription)
494498
try {
495499
if (targetWebhookId) {
496500
logger.info(`[${requestId}] Updating existing webhook for path: ${finalPath}`, {
497501
webhookId: targetWebhookId,
498502
provider,
499-
hasCredentialId: !!(resolvedProviderConfig as any)?.credentialId,
500-
credentialId: (resolvedProviderConfig as any)?.credentialId,
503+
hasCredentialId: !!(configToSave as any)?.credentialId,
504+
credentialId: (configToSave as any)?.credentialId,
501505
})
502506
const updatedResult = await db
503507
.update(webhook)
504508
.set({
505509
blockId,
506510
provider,
507-
providerConfig: resolvedProviderConfig,
511+
providerConfig: configToSave,
508512
credentialSetId:
509-
((resolvedProviderConfig as Record<string, unknown>)?.credentialSetId as
510-
| string
511-
| null) || null,
513+
((configToSave as Record<string, unknown>)?.credentialSetId as string | null) || null,
512514
isActive: true,
513515
updatedAt: new Date(),
514516
})
@@ -531,11 +533,9 @@ export async function POST(request: NextRequest) {
531533
blockId,
532534
path: finalPath,
533535
provider,
534-
providerConfig: resolvedProviderConfig,
536+
providerConfig: configToSave,
535537
credentialSetId:
536-
((resolvedProviderConfig as Record<string, unknown>)?.credentialSetId as
537-
| string
538-
| null) || null,
538+
((configToSave as Record<string, unknown>)?.credentialSetId as string | null) || null,
539539
isActive: true,
540540
createdAt: new Date(),
541541
updatedAt: new Date(),
@@ -549,7 +549,7 @@ export async function POST(request: NextRequest) {
549549
try {
550550
const { cleanupExternalWebhook } = await import('@/lib/webhooks/provider-subscriptions')
551551
await cleanupExternalWebhook(
552-
createTempWebhookData(resolvedProviderConfig),
552+
createTempWebhookData(configToSave),
553553
workflowRecord,
554554
requestId
555555
)

apps/sim/app/api/workflows/[id]/execute/route.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ type AsyncExecutionParams = {
116116
userId: string
117117
input: any
118118
triggerType: CoreTriggerType
119-
preflighted?: boolean
120119
}
121120

122121
/**
@@ -139,7 +138,6 @@ async function handleAsyncExecution(params: AsyncExecutionParams): Promise<NextR
139138
userId,
140139
input,
141140
triggerType,
142-
preflighted: params.preflighted,
143141
}
144142

145143
try {
@@ -276,7 +274,6 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
276274
requestId
277275
)
278276

279-
const shouldPreflightEnvVars = isAsyncMode && isTriggerDevEnabled
280277
const preprocessResult = await preprocessExecution({
281278
workflowId,
282279
userId,
@@ -285,9 +282,7 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
285282
requestId,
286283
checkDeployment: !shouldUseDraftState,
287284
loggingSession,
288-
preflightEnvVars: shouldPreflightEnvVars,
289285
useDraftState: shouldUseDraftState,
290-
envUserId: isClientSession ? userId : undefined,
291286
})
292287

293288
if (!preprocessResult.success) {
@@ -319,7 +314,6 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
319314
userId: actorUserId,
320315
input,
321316
triggerType: loggingTriggerType,
322-
preflighted: shouldPreflightEnvVars,
323317
})
324318
}
325319

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/code/code.tsx

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import type { GenerationType } from '@/blocks/types'
3838
import { normalizeName } from '@/executor/constants'
3939
import { createEnvVarPattern, createReferencePattern } from '@/executor/utils/reference-validation'
4040
import { useTagSelection } from '@/hooks/kb/use-tag-selection'
41+
import { createShouldHighlightEnvVar, useAvailableEnvVarKeys } from '@/hooks/use-available-env-vars'
4142

4243
const logger = createLogger('Code')
4344

@@ -88,21 +89,27 @@ interface CodePlaceholder {
8889
/**
8990
* Creates a syntax highlighter function with custom reference and environment variable highlighting.
9091
* @param effectiveLanguage - The language to use for syntax highlighting
91-
* @param shouldHighlightReference - Function to determine if a reference should be highlighted
92+
* @param shouldHighlightReference - Function to determine if a block reference should be highlighted
93+
* @param shouldHighlightEnvVar - Function to determine if an env var should be highlighted
9294
* @returns A function that highlights code with syntax and custom highlights
9395
*/
9496
const createHighlightFunction = (
9597
effectiveLanguage: 'javascript' | 'python' | 'json',
96-
shouldHighlightReference: (part: string) => boolean
98+
shouldHighlightReference: (part: string) => boolean,
99+
shouldHighlightEnvVar: (varName: string) => boolean
97100
) => {
98101
return (codeToHighlight: string): string => {
99102
const placeholders: CodePlaceholder[] = []
100103
let processedCode = codeToHighlight
101104

102105
processedCode = processedCode.replace(createEnvVarPattern(), (match) => {
103-
const placeholder = `__ENV_VAR_${placeholders.length}__`
104-
placeholders.push({ placeholder, original: match, type: 'env' })
105-
return placeholder
106+
const varName = match.slice(2, -2).trim()
107+
if (shouldHighlightEnvVar(varName)) {
108+
const placeholder = `__ENV_VAR_${placeholders.length}__`
109+
placeholders.push({ placeholder, original: match, type: 'env' })
110+
return placeholder
111+
}
112+
return match
106113
})
107114

108115
processedCode = processedCode.replace(createReferencePattern(), (match) => {
@@ -212,6 +219,7 @@ export const Code = memo(function Code({
212219
const accessiblePrefixes = useAccessibleReferencePrefixes(blockId)
213220
const emitTagSelection = useTagSelection(blockId, subBlockId)
214221
const [languageValue] = useSubBlockValue<string>(blockId, 'language')
222+
const availableEnvVars = useAvailableEnvVarKeys(workspaceId)
215223

216224
const effectiveLanguage = (languageValue as 'javascript' | 'python' | 'json') || language
217225

@@ -603,9 +611,15 @@ export const Code = memo(function Code({
603611
[generateCodeStream, isPromptVisible, isAiStreaming]
604612
)
605613

614+
const shouldHighlightEnvVar = useMemo(
615+
() => createShouldHighlightEnvVar(availableEnvVars),
616+
[availableEnvVars]
617+
)
618+
606619
const highlightCode = useMemo(
607-
() => createHighlightFunction(effectiveLanguage, shouldHighlightReference),
608-
[effectiveLanguage, shouldHighlightReference]
620+
() =>
621+
createHighlightFunction(effectiveLanguage, shouldHighlightReference, shouldHighlightEnvVar),
622+
[effectiveLanguage, shouldHighlightReference, shouldHighlightEnvVar]
609623
)
610624

611625
const handleValueChange = useCallback(

0 commit comments

Comments
 (0)