Skip to content

Commit f9b09ec

Browse files
authored
fix(webhooks): resolve env var references before deploy-triggered subscription creation (#5619)
* fix(webhooks): resolve env var references before deploy-triggered subscription creation Provider config fields like an API key can reference an environment variable via {{VAR_NAME}}. The interactive trigger-save route already resolved these before calling a provider's createSubscription, but the async deployment-outbox path (workflow deploy -> saveTriggerWebhooksForDeploy -> createExternalWebhookSubscription) did not, so the literal unresolved {{VAR_NAME}} string was sent to the provider as the credential and rejected. Resolve env vars in createExternalWebhookSubscription itself so both callers behave the same; the persisted providerConfig keeps storing the unresolved template, only the outbound call gets the resolved value. * fix(webhooks): guard against a non-string workspaceId when resolving env vars workflow.workspaceId as string | undefined was an unchecked cast on a Record<string, unknown> — if a caller ever passed a workflow-like object where workspaceId isn't actually a string, workspace-scoped {{VAR}} references would silently stay unresolved and the provider would receive the literal template as the credential, reproducing the exact class of bug this change exists to fix. Replaced with a runtime typeof check that falls back to undefined (personal-env-only resolution) instead of forwarding an unvalidated value.
1 parent 1fe94d3 commit f9b09ec

2 files changed

Lines changed: 139 additions & 1 deletion

File tree

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { NextRequest } from 'next/server'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mockGetEffectiveDecryptedEnv, mockGetProviderHandler } = vi.hoisted(() => ({
8+
mockGetEffectiveDecryptedEnv: vi.fn(),
9+
mockGetProviderHandler: vi.fn(),
10+
}))
11+
12+
vi.mock('@/lib/environment/utils', () => ({
13+
getEffectiveDecryptedEnv: mockGetEffectiveDecryptedEnv,
14+
}))
15+
16+
vi.mock('@/lib/webhooks/providers', () => ({
17+
getProviderHandler: mockGetProviderHandler,
18+
}))
19+
20+
import { createExternalWebhookSubscription } from '@/lib/webhooks/provider-subscriptions'
21+
22+
describe('createExternalWebhookSubscription', () => {
23+
beforeEach(() => {
24+
vi.clearAllMocks()
25+
mockGetEffectiveDecryptedEnv.mockResolvedValue({ ASHBY_API_KEY: 'real-secret-key' })
26+
})
27+
28+
it('resolves {{ENV_VAR}} references in providerConfig before calling the provider', async () => {
29+
const createSubscription = vi.fn().mockResolvedValue({
30+
providerConfigUpdates: { externalId: 'ext-1' },
31+
})
32+
mockGetProviderHandler.mockReturnValue({ createSubscription })
33+
34+
const webhookData = {
35+
provider: 'ashby',
36+
providerConfig: { apiKey: '{{ASHBY_API_KEY}}', triggerId: 'ashby_application_submit' },
37+
}
38+
const workflow = { id: 'wf-1', workspaceId: 'ws-1' }
39+
40+
await createExternalWebhookSubscription(
41+
{} as NextRequest,
42+
webhookData,
43+
workflow,
44+
'user-1',
45+
'req-1'
46+
)
47+
48+
expect(mockGetEffectiveDecryptedEnv).toHaveBeenCalledWith('user-1', 'ws-1')
49+
const passedWebhook = createSubscription.mock.calls[0][0].webhook
50+
expect(passedWebhook.providerConfig.apiKey).toBe('real-secret-key')
51+
})
52+
53+
it('persists the unresolved providerConfig, not the resolved one, back to the caller', async () => {
54+
const createSubscription = vi.fn().mockResolvedValue({
55+
providerConfigUpdates: { externalId: 'ext-1' },
56+
})
57+
mockGetProviderHandler.mockReturnValue({ createSubscription })
58+
59+
const webhookData = {
60+
provider: 'ashby',
61+
providerConfig: { apiKey: '{{ASHBY_API_KEY}}', triggerId: 'ashby_application_submit' },
62+
}
63+
const workflow = { id: 'wf-1', workspaceId: 'ws-1' }
64+
65+
const result = await createExternalWebhookSubscription(
66+
{} as NextRequest,
67+
webhookData,
68+
workflow,
69+
'user-1',
70+
'req-1'
71+
)
72+
73+
expect(result.updatedProviderConfig.apiKey).toBe('{{ASHBY_API_KEY}}')
74+
expect(result.updatedProviderConfig.externalId).toBe('ext-1')
75+
})
76+
77+
it('falls back to personal-only env resolution when workspaceId is not a string', async () => {
78+
const createSubscription = vi.fn().mockResolvedValue({
79+
providerConfigUpdates: { externalId: 'ext-1' },
80+
})
81+
mockGetProviderHandler.mockReturnValue({ createSubscription })
82+
83+
const webhookData = {
84+
provider: 'ashby',
85+
providerConfig: { apiKey: '{{ASHBY_API_KEY}}', triggerId: 'ashby_application_submit' },
86+
}
87+
const workflow = { id: 'wf-1', workspaceId: null }
88+
89+
await createExternalWebhookSubscription(
90+
{} as NextRequest,
91+
webhookData,
92+
workflow,
93+
'user-1',
94+
'req-1'
95+
)
96+
97+
expect(mockGetEffectiveDecryptedEnv).toHaveBeenCalledWith('user-1', undefined)
98+
})
99+
100+
it('skips resolution and provider call entirely when the provider has no createSubscription', async () => {
101+
mockGetProviderHandler.mockReturnValue({})
102+
103+
const webhookData = {
104+
provider: 'slack',
105+
providerConfig: { token: '{{SLACK_TOKEN}}' },
106+
}
107+
const workflow = { id: 'wf-1', workspaceId: 'ws-1' }
108+
109+
const result = await createExternalWebhookSubscription(
110+
{} as NextRequest,
111+
webhookData,
112+
workflow,
113+
'user-1',
114+
'req-1'
115+
)
116+
117+
expect(mockGetEffectiveDecryptedEnv).not.toHaveBeenCalled()
118+
expect(result.externalSubscriptionCreated).toBe(false)
119+
expect(result.updatedProviderConfig.token).toBe('{{SLACK_TOKEN}}')
120+
})
121+
})

apps/sim/lib/webhooks/provider-subscriptions.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
33
import type { NextRequest } from 'next/server'
4+
import { resolveWebhookProviderConfig } from '@/lib/webhooks/env-resolver'
45
import { getProviderHandler } from '@/lib/webhooks/providers'
56

67
const logger = createLogger('WebhookProviderSubscriptions')
@@ -88,6 +89,14 @@ export function shouldRecreateExternalWebhookSubscription({
8889
* Ask the provider handler to create an external webhook subscription, if that
8990
* provider supports automatic registration.
9091
*
92+
* `providerConfig` may contain unresolved `{{ENV_VAR}}` references (e.g. an
93+
* API key field backed by an environment variable) — these are resolved here
94+
* before the provider call so deploy-triggered registration (this function is
95+
* also called from the async deployment outbox, not just the interactive
96+
* webhook-save route) behaves the same as a manual save. The persisted
97+
* `providerConfig` returned to the caller stays unresolved; only the
98+
* provider-managed fields from `result.providerConfigUpdates` get merged in.
99+
*
91100
* The returned provider-managed fields are merged back into `providerConfig`
92101
* by the caller.
93102
*/
@@ -106,8 +115,16 @@ export async function createExternalWebhookSubscription(
106115
return { updatedProviderConfig: providerConfig, externalSubscriptionCreated: false }
107116
}
108117

118+
const workspaceId = typeof workflow.workspaceId === 'string' ? workflow.workspaceId : undefined
119+
120+
const resolvedProviderConfig = await resolveWebhookProviderConfig(
121+
providerConfig,
122+
userId,
123+
workspaceId
124+
)
125+
109126
const result = await handler.createSubscription({
110-
webhook: webhookData,
127+
webhook: { ...webhookData, providerConfig: resolvedProviderConfig },
111128
workflow,
112129
userId,
113130
requestId,

0 commit comments

Comments
 (0)