Skip to content

Commit 5fc969c

Browse files
committed
fix(telegram): match the active bot with the env the caller resolved its token with
Subscription creation resolves the incoming bot token with the deployer's env, while cleanup resolves with the background env; the active-row matcher now uses the same env as its caller, so a bot referenced through a personal variable still reuses the active secret and is not deleted from under the active deployment. Tests: the idempotency service gets one central mock in @sim/testing, used by every test that mocked it locally, and the new tests import single factory and mock files instead of the @sim/testing barrel.
1 parent 3a141dd commit 5fc969c

11 files changed

Lines changed: 141 additions & 73 deletions

File tree

‎apps/sim/lib/billing/checkout-admission.test.ts‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,19 @@
1+
import {
2+
idempotencyServiceMock,
3+
idempotencyServiceMockFns,
4+
} from '@sim/testing/mocks/idempotency-service.mock'
15
import { beforeEach, describe, expect, it, vi } from 'vitest'
26

3-
const { mockAtomicallyClaim, mockRelease, mockIdempotencyService } = vi.hoisted(() => ({
4-
mockAtomicallyClaim: vi.fn(),
5-
mockRelease: vi.fn(),
6-
mockIdempotencyService: vi.fn(),
7-
}))
8-
9-
vi.mock('@/lib/core/idempotency/service', () => ({
10-
IdempotencyService: class MockIdempotencyService {
11-
constructor(options: unknown) {
12-
mockIdempotencyService(options)
13-
}
14-
15-
atomicallyClaim(...args: unknown[]) {
16-
return mockAtomicallyClaim(...args)
17-
}
18-
19-
release(...args: unknown[]) {
20-
return mockRelease(...args)
21-
}
22-
},
23-
}))
7+
vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
248

259
import {
2610
claimCheckoutAdmission,
2711
releaseCheckoutAdmission,
2812
resolveCheckoutReferenceId,
2913
} from '@/lib/billing/checkout-admission'
3014

15+
const { mockAtomicallyClaim, mockRelease } = idempotencyServiceMockFns
16+
3117
describe('checkout admission', () => {
3218
beforeEach(() => {
3319
mockAtomicallyClaim.mockReset()

‎apps/sim/lib/execution/preprocessing.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { loggingSessionMock, loggingSessionMockFns, workflowAuthzMockFns } from '@sim/testing'
1+
import { loggingSessionMock, workflowAuthzMockFns } from '@sim/testing'
22
import { authBanMock, authBanMockFns } from '@sim/testing/mocks/auth-ban.mock'
33
import {
44
billingAttributionMock,
@@ -15,6 +15,7 @@ import {
1515
billingUsageReservationMockFns,
1616
} from '@sim/testing/mocks/billing-usage-reservation.mock'
1717
import { executionLimitsMock } from '@sim/testing/mocks/execution-limits.mock'
18+
import { loggingSessionMockFns } from '@sim/testing/mocks/logging-session.mock'
1819
import { createMockRedis } from '@sim/testing/mocks/redis.mock'
1920
import { redisConfigMockFns, resetRedisConfigMock } from '@sim/testing/mocks/redis-config.mock'
2021
import { utilsHelpersMock } from '@sim/testing/mocks/utils-helpers.mock'

‎apps/sim/lib/webhooks/polling/google-calendar.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
2-
import { createWorkflowRecord } from '@sim/testing'
2+
import { createWorkflowRecord } from '@sim/testing/factories/permission.factory'
33
import { jsonResponse } from '@sim/testing/helpers/http'
4+
import { idempotencyServiceMock } from '@sim/testing/mocks/idempotency-service.mock'
45
import {
56
webhooksPollingUtilsMock,
67
webhooksPollingUtilsMockFns,
@@ -11,13 +12,7 @@ import {
1112
} from '@sim/testing/mocks/webhooks-processor.mock'
1213
import { beforeEach, describe, expect, it, vi } from 'vitest'
1314

14-
vi.mock('@/lib/core/idempotency/service', () => ({
15-
pollingIdempotency: {
16-
executeWithIdempotency: vi.fn(
17-
async (_provider: string, _key: string, execute: () => Promise<unknown>) => execute()
18-
),
19-
},
20-
}))
15+
vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
2116

2217
vi.mock('@/lib/webhooks/processor', () => webhooksProcessorMock)
2318

‎apps/sim/lib/webhooks/polling/imap.test.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { dbChainMockFns } from '@sim/testing'
2+
import { idempotencyServiceMock } from '@sim/testing/mocks/idempotency-service.mock'
23
import {
34
webhooksPollingUtilsMock,
45
webhooksPollingUtilsMockFns,
@@ -18,9 +19,7 @@ const {
1819
mockResolveImapConnectionForActor: vi.fn(),
1920
}))
2021

21-
vi.mock('@/lib/core/idempotency/service', () => ({
22-
pollingIdempotency: { executeWithIdempotency: vi.fn() },
23-
}))
22+
vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
2423

2524
vi.mock('@/lib/imap/connection.server', () => ({
2625
createSecureImapClient: mockCreateSecureImapClient,

‎apps/sim/lib/webhooks/polling/orchestrator.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { webhook } from '@sim/db/schema'
2-
import { createWorkflowRecord } from '@sim/testing'
2+
import { createWorkflowRecord } from '@sim/testing/factories/permission.factory'
33
import { queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
44
import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing/mocks/env-flags.mock'
55
import { createMockRedis } from '@sim/testing/mocks/redis.mock'

‎apps/sim/lib/webhooks/polling/rss.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
2-
import { createWorkflowRecord } from '@sim/testing'
2+
import { createWorkflowRecord } from '@sim/testing/factories/permission.factory'
3+
import { idempotencyServiceMock } from '@sim/testing/mocks/idempotency-service.mock'
34
import {
45
inputValidationMock,
56
inputValidationMockFns,
@@ -18,13 +19,7 @@ vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock
1819
const mockFetch = inputValidationMockFns.mockSecureFetchWithPinnedIP
1920
const mockValidateUrl = inputValidationMockFns.mockValidateUrlWithDNS
2021

21-
vi.mock('@/lib/core/idempotency/service', () => ({
22-
pollingIdempotency: {
23-
executeWithIdempotency: vi.fn(
24-
async (_provider: string, _key: string, execute: () => Promise<unknown>) => execute()
25-
),
26-
},
27-
}))
22+
vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
2823

2924
vi.mock('@/lib/webhooks/processor', () => webhooksProcessorMock)
3025

‎apps/sim/lib/webhooks/providers/telegram.test.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,17 @@ describe('Telegram bot tokens stored as environment variable references', () =>
115115
beforeEach(() => {
116116
resetDbChainMock()
117117
billingAttributionMockFns.mockGetWorkspaceBilledAccountUserId.mockResolvedValue('owner-1')
118-
environmentUtilsMockFns.mockGetExecutionEnvironment.mockResolvedValue({
119-
personalDecrypted: {},
120-
workspaceDecrypted: { TELEGRAM_BOT_TOKEN: BOT_TOKEN },
121-
})
122118
queueTableRows(webhook, [{ id: 'active-row', providerConfig: activeConfig }])
123119
})
124120

125-
it('reuses the active secret when the active row stores the token as a reference', async () => {
121+
it("reuses the active secret when the reference resolves in the deployer's env", async () => {
122+
environmentUtilsMockFns.mockGetEffectiveDecryptedEnv.mockResolvedValue({
123+
TELEGRAM_BOT_TOKEN: BOT_TOKEN,
124+
})
125+
environmentUtilsMockFns.mockGetExecutionEnvironment.mockResolvedValue({
126+
personalDecrypted: {},
127+
workspaceDecrypted: {},
128+
})
126129
const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ ok: true, result: true }, 200))
127130
vi.stubGlobal('fetch', fetchMock)
128131

@@ -139,6 +142,10 @@ describe('Telegram bot tokens stored as environment variable references', () =>
139142
})
140143

141144
it('leaves the bot webhook in place when the active deployment uses the same referenced bot', async () => {
145+
environmentUtilsMockFns.mockGetExecutionEnvironment.mockResolvedValue({
146+
personalDecrypted: {},
147+
workspaceDecrypted: { TELEGRAM_BOT_TOKEN: BOT_TOKEN },
148+
})
142149
const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ ok: true, result: true }, 200))
143150
vi.stubGlobal('fetch', fetchMock)
144151

‎apps/sim/lib/webhooks/providers/telegram.ts‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getErrorMessage } from '@sim/utils/errors'
44
import { generateShortId } from '@sim/utils/id'
55
import { and, eq, isNull, ne } from 'drizzle-orm'
66
import { NextResponse } from 'next/server'
7+
import { getEffectiveDecryptedEnv } from '@/lib/environment/utils'
78
import { resolveBackgroundWebhookEnv } from '@/lib/webhooks/env-resolver'
89
import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils'
910
import type {
@@ -221,7 +222,8 @@ export const telegramHandler: WebhookProviderHandler = {
221222
const activeConfigs = await findActiveTelegramConfigsForBot(
222223
ctx.webhook.id,
223224
ctx.workflow,
224-
botToken
225+
botToken,
226+
() => backgroundEnvFor(ctx.workflow)
225227
)
226228
if (activeConfigs.length > 0) {
227229
logger.info(
@@ -274,10 +276,12 @@ async function resolveSubscriptionSecretToken(
274276
const ownSecret = readSecretToken(config)
275277
if (ownSecret) return ownSecret
276278

279+
const workspaceId = workspaceIdOf(ctx.workflow)
277280
const activeConfigs = await findActiveTelegramConfigsForBot(
278281
ctx.webhook.id,
279282
ctx.workflow,
280-
botToken
283+
botToken,
284+
() => getEffectiveDecryptedEnv(ctx.userId, workspaceId)
281285
)
282286
for (const activeConfig of activeConfigs) {
283287
const activeSecret = readSecretToken(activeConfig)
@@ -287,16 +291,30 @@ async function resolveSubscriptionSecretToken(
287291
return generateShortId(TELEGRAM_SECRET_TOKEN_LENGTH)
288292
}
289293

294+
function workspaceIdOf(workflowRecord: Record<string, unknown>): string | undefined {
295+
return typeof workflowRecord.workspaceId === 'string' ? workflowRecord.workspaceId : undefined
296+
}
297+
298+
/** The env cleanup resolves a stored config with (`cleanupExternalWebhook`). */
299+
async function backgroundEnvFor(workflowRecord: Record<string, unknown>) {
300+
const ownerUserId = workflowRecord.userId
301+
return typeof ownerUserId === 'string'
302+
? resolveBackgroundWebhookEnv(ownerUserId, workspaceIdOf(workflowRecord))
303+
: {}
304+
}
305+
290306
/**
291307
* Provider configs of other active-deployment Telegram webhooks in the workflow
292308
* using `botToken`. Rows store the bot token as authored, often a `{{VAR}}`
293-
* reference, while subscription callers hold it resolved, so each stored token
294-
* is resolved against the same background env before comparing.
309+
* reference, while the caller holds it resolved, so each stored token is
310+
* resolved with `loadEnv` — the same env the caller resolved its own token with —
311+
* before comparing.
295312
*/
296313
async function findActiveTelegramConfigsForBot(
297314
webhookId: unknown,
298315
workflowRecord: Record<string, unknown>,
299-
botToken: string
316+
botToken: string,
317+
loadEnv: () => Promise<Record<string, string>>
300318
): Promise<Record<string, unknown>[]> {
301319
const workflowId = workflowRecord.id
302320
if (typeof workflowId !== 'string' || typeof webhookId !== 'string') return []
@@ -325,14 +343,7 @@ async function findActiveTelegramConfigsForBot(
325343
const referencesEnv = activeConfigs.some((config) =>
326344
createEnvVarPattern().test(String(config.botToken ?? ''))
327345
)
328-
const ownerUserId = workflowRecord.userId
329-
const envVars =
330-
referencesEnv && typeof ownerUserId === 'string'
331-
? await resolveBackgroundWebhookEnv(
332-
ownerUserId,
333-
typeof workflowRecord.workspaceId === 'string' ? workflowRecord.workspaceId : undefined
334-
)
335-
: {}
346+
const envVars = referencesEnv ? await loadEnv() : {}
336347
return activeConfigs.filter(
337348
(config) => resolveEnvVarReferences(config.botToken, envVars) === botToken
338349
)

‎apps/sim/lib/workspace-files/application/extract-workspace-file.test.ts‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import {
55
createSessionPrincipal,
66
createWorkspaceApiKeyPrincipal,
77
} from '@sim/testing/factories/principal.factory'
8+
import {
9+
idempotencyServiceMock,
10+
idempotencyServiceMockFns,
11+
} from '@sim/testing/mocks/idempotency-service.mock'
812
import { realtimeNotifyMock, realtimeNotifyMockFns } from '@sim/testing/mocks/realtime-notify.mock'
913
import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
1014
import {
@@ -23,26 +27,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
2327
import { OrchestrationError } from '@/lib/core/orchestration/types'
2428

2529
const hoisted = vi.hoisted(() => ({
26-
atomicallyClaim: vi.fn(),
2730
decompress: vi.fn(),
28-
releaseLease: vi.fn(),
2931
}))
3032

3133
vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
3234

3335
vi.mock('@/lib/realtime/notify', () => realtimeNotifyMock)
3436

35-
vi.mock('@/lib/core/idempotency/service', () => ({
36-
IdempotencyService: class MockIdempotencyService {
37-
atomicallyClaim(...args: unknown[]) {
38-
return hoisted.atomicallyClaim(...args)
39-
}
40-
41-
release(...args: unknown[]) {
42-
return hoisted.releaseLease(...args)
43-
}
44-
},
45-
}))
37+
vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
4638

4739
vi.mock('@/lib/uploads/archive', () => ({
4840
decompressArchiveBufferToWorkspaceFiles: hoisted.decompress,
@@ -71,6 +63,8 @@ const mocks = {
7163
loadContext: workspaceFileManagerMockFns.mockLoadActiveWorkspaceFileContext,
7264
notify: realtimeNotifyMockFns.mockNotifyWorkspaceFilesChanged,
7365
...hoisted,
66+
atomicallyClaim: idempotencyServiceMockFns.mockAtomicallyClaim,
67+
releaseLease: idempotencyServiceMockFns.mockRelease,
7468
createFolder: workspaceFileFoldersMockFns.mockCreateWorkspaceFileFolder,
7569
archiveFolderIfEmpty: workspaceFileFoldersMockFns.mockArchiveWorkspaceFileFolderIfEmpty,
7670
resolvePermission: workspaceAuthzMockFns.mockResolveEffectiveWorkspacePermission,
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { vi } from 'vitest'
2+
3+
/**
4+
* Controllable mock functions for `@/lib/core/idempotency/service`.
5+
*
6+
* Every `IdempotencyService` instance and the exported singletons (`webhookIdempotency`,
7+
* `pollingIdempotency`, `chatSendIdempotency`) share these functions. `executeWithIdempotency`
8+
* runs the operation once, as a first claim would; `executeOrSkipInProgress` resolves with its
9+
* result. `atomicallyClaim` is a bare `vi.fn()`; `release` resolves `undefined`.
10+
* `mockConstructor` records each `new IdempotencyService(config)`. Defaults are
11+
* `vi.fn(impl)`, so `mockReset()` restores them.
12+
*
13+
* @example
14+
* ```ts
15+
* import { idempotencyServiceMockFns } from '@sim/testing/mocks/idempotency-service.mock'
16+
*
17+
* idempotencyServiceMockFns.mockAtomicallyClaim.mockResolvedValue({ claimed: true })
18+
* ```
19+
*/
20+
export const idempotencyServiceMockFns = {
21+
mockConstructor: vi.fn((_config?: unknown): void => {}),
22+
mockAtomicallyClaim: vi.fn(),
23+
mockRelease: vi.fn(async (..._args: unknown[]): Promise<void> => {}),
24+
mockExecuteWithIdempotency: vi.fn(
25+
async (_provider: string, _identifier: string, operation: () => Promise<unknown>) => operation()
26+
),
27+
mockExecuteOrSkipInProgress: vi.fn(
28+
async (_provider: string, _identifier: string, operation: () => Promise<unknown>) => ({
29+
outcome: 'resolved' as const,
30+
result: await operation(),
31+
})
32+
),
33+
}
34+
35+
const idempotencyMethods = {
36+
atomicallyClaim: (...args: unknown[]) => idempotencyServiceMockFns.mockAtomicallyClaim(...args),
37+
release: (...args: unknown[]) => idempotencyServiceMockFns.mockRelease(...args),
38+
executeWithIdempotency: (
39+
provider: string,
40+
identifier: string,
41+
operation: () => Promise<unknown>
42+
) => idempotencyServiceMockFns.mockExecuteWithIdempotency(provider, identifier, operation),
43+
executeOrSkipInProgress: (
44+
provider: string,
45+
identifier: string,
46+
operation: () => Promise<unknown>
47+
) => idempotencyServiceMockFns.mockExecuteOrSkipInProgress(provider, identifier, operation),
48+
}
49+
50+
class IdempotencyService {
51+
constructor(config?: unknown) {
52+
idempotencyServiceMockFns.mockConstructor(config)
53+
}
54+
55+
atomicallyClaim = idempotencyMethods.atomicallyClaim
56+
release = idempotencyMethods.release
57+
executeWithIdempotency = idempotencyMethods.executeWithIdempotency
58+
executeOrSkipInProgress = idempotencyMethods.executeOrSkipInProgress
59+
}
60+
61+
/**
62+
* Static mock module for `@/lib/core/idempotency/service`. Covers every runtime export;
63+
* `WEBHOOK_IN_PROGRESS_LEASE_SECONDS` carries the real value (2 hours).
64+
*
65+
* @example
66+
* ```ts
67+
* vi.mock('@/lib/core/idempotency/service', () => idempotencyServiceMock)
68+
* ```
69+
*/
70+
export const idempotencyServiceMock = {
71+
WEBHOOK_IN_PROGRESS_LEASE_SECONDS: 60 * 60 * 2,
72+
IdempotencyService,
73+
webhookIdempotency: idempotencyMethods,
74+
pollingIdempotency: idempotencyMethods,
75+
chatSendIdempotency: idempotencyMethods,
76+
}

0 commit comments

Comments
 (0)