Skip to content

Commit 78f665f

Browse files
icecrasher321claude
andcommitted
improvement(webhooks): resolve the webhook credential once, off the critical path
The worker starts resolveCredentialAccountUserId before the state/webhook loads and awaits it only where the owner id is first consumed, so its two serial reads overlap the deployment-state load and provider-config resolution. The resolved owner rides into formatInput as credentialOwnerUserId (when the provider config names the same credential), letting the Slack handler skip re-running the identical resolveOAuthAccountId + account owner chain before refreshing the token. The Slack token resolution is extracted into resolveSlackWebhookBotToken, shared and behavior-identical when no owner id is provided. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b504a20 commit 78f665f

4 files changed

Lines changed: 123 additions & 36 deletions

File tree

apps/sim/background/webhook-execution.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,40 @@ describe('executeWebhookJob fault vs error handling', () => {
389389
)
390390
})
391391

392+
it('forwards the resolved credential owner into formatInput when the credential matches', async () => {
393+
const formatInput = vi.fn().mockResolvedValue({ input: { event: {} } })
394+
mockGetProviderHandler.mockReturnValue({ formatInput })
395+
const { resolveOAuthAccountId } = await import('@/lib/oauth/credential-service')
396+
vi.mocked(resolveOAuthAccountId).mockResolvedValue({
397+
accountId: 'account-1',
398+
} as never)
399+
dbChainMockFns.limit.mockResolvedValue([{ userId: 'owner-1' }])
400+
mockExecuteWorkflowCore.mockResolvedValue({
401+
success: true,
402+
status: 'completed',
403+
output: {},
404+
logs: [],
405+
executionState: {
406+
blockStates: {},
407+
executedBlocks: [],
408+
blockLogs: [],
409+
decisions: {},
410+
completedLoops: [],
411+
activeExecutionPath: [],
412+
},
413+
})
414+
const warmContext = {
415+
workflowRecord: { id: 'workflow-1', workspaceId: 'workspace-1', userId: 'user-1' },
416+
webhookRecord: { id: 'webhook-1', providerConfig: { credentialId: 'credential-1' } },
417+
} as unknown as NonNullable<Parameters<typeof executeWebhookJob>[2]>
418+
419+
await executeWebhookJob({ ...payload, credentialId: 'credential-1' }, undefined, warmContext)
420+
421+
expect(formatInput).toHaveBeenCalledWith(
422+
expect.objectContaining({ credentialOwnerUserId: 'owner-1' })
423+
)
424+
})
425+
392426
it('loads rows and keeps account checks without warm context', async () => {
393427
mockExecuteWorkflowCore.mockResolvedValue({
394428
success: true,

apps/sim/background/webhook-execution.ts

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -580,22 +580,23 @@ async function executeWebhookJobInternal(
580580
: loadDeployedWorkflowState(payload.workflowId, workspaceId)
581581
const warmWebhookRecord =
582582
warmContext?.webhookRecord?.id === payload.webhookId ? warmContext.webhookRecord : undefined
583-
const [workflowData, webhookRows, resolvedCredentialUserId] = await Promise.all([
583+
/**
584+
* Started here, awaited only where the owner id is first needed (formatInput),
585+
* so its two serial reads overlap the state load and provider-config
586+
* resolution instead of gating them. The empty catch marks the chain observed
587+
* for the gap; the later await still surfaces the real error.
588+
*/
589+
const credentialAccountUserIdPromise = payload.credentialId
590+
? resolveCredentialAccountUserId(payload.credentialId)
591+
: Promise.resolve(undefined)
592+
credentialAccountUserIdPromise.catch(() => {})
593+
const [workflowData, webhookRows] = await Promise.all([
584594
workflowStatePromise,
585595
warmWebhookRecord
586596
? Promise.resolve([warmWebhookRecord])
587597
: db.select().from(webhook).where(eq(webhook.id, payload.webhookId)).limit(1),
588-
payload.credentialId
589-
? resolveCredentialAccountUserId(payload.credentialId)
590-
: Promise.resolve(undefined),
591598
])
592599
const loadsEndedAt = Date.now()
593-
const credentialAccountUserId = resolvedCredentialUserId
594-
if (payload.credentialId && !credentialAccountUserId) {
595-
logger.warn(
596-
`[${requestId}] Failed to resolve credential account for credential ${payload.credentialId}`
597-
)
598-
}
599600

600601
if (!workflowData) {
601602
throw new Error(
@@ -654,6 +655,20 @@ async function executeWebhookJobInternal(
654655
)
655656
const providerConfigEndedAt = Date.now()
656657

658+
const credentialAccountUserId = await credentialAccountUserIdPromise
659+
if (payload.credentialId && !credentialAccountUserId) {
660+
logger.warn(
661+
`[${requestId}] Failed to resolve credential account for credential ${payload.credentialId}`
662+
)
663+
}
664+
const resolvedProviderConfig = resolvedWebhookRecord.providerConfig
665+
const formatInputCredentialOwnerUserId =
666+
credentialAccountUserId &&
667+
payload.credentialId &&
668+
resolvedProviderConfig.credentialId === payload.credentialId
669+
? credentialAccountUserId
670+
: undefined
671+
657672
if (handler.formatInput) {
658673
const result = await handler.formatInput({
659674
webhook: resolvedWebhookRecord,
@@ -663,6 +678,9 @@ async function executeWebhookJobInternal(
663678
query: payload.query ?? {},
664679
method: payload.method ?? '',
665680
requestId,
681+
...(formatInputCredentialOwnerUserId
682+
? { credentialOwnerUserId: formatInputCredentialOwnerUserId }
683+
: {}),
666684
})
667685
input = result.input as Record<string, unknown> | null
668686
skipMessage = result.skip?.message

apps/sim/lib/webhooks/providers/slack.ts

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,50 @@ export function shouldSkipSlackTriggerEvent(
808808
return false
809809
}
810810

811+
/**
812+
* Resolves the bot token a Slack webhook can act with, across the three trigger
813+
* backends: a pasted bot token (legacy `slack_webhook`), a reusable custom-bot
814+
* credential's stored token, and a native `slack_app` OAuth credential. The
815+
* OAuth credential resolves via its OWNER (not the execution actor in
816+
* `workflow.userId`, who may not own the credential) so reaction-message text
817+
* and file downloads work. `credentialOwnerUserId` short-circuits the
818+
* credential → account → owner chain when the caller already resolved it.
819+
*/
820+
async function resolveSlackWebhookBotToken(
821+
providerConfig: Record<string, unknown>,
822+
requestId: string,
823+
credentialOwnerUserId?: string
824+
): Promise<string | undefined> {
825+
const pastedToken = providerConfig.botToken as string | undefined
826+
if (pastedToken || typeof providerConfig.credentialId !== 'string') {
827+
return pastedToken
828+
}
829+
const credentialId = providerConfig.credentialId
830+
831+
const botCredential = await getSlackBotCredential(credentialId)
832+
if (botCredential?.botToken) {
833+
return botCredential.botToken
834+
}
835+
836+
let ownerUserId = credentialOwnerUserId
837+
if (!ownerUserId) {
838+
const resolved = await resolveOAuthAccountId(credentialId)
839+
if (!resolved?.accountId) {
840+
return undefined
841+
}
842+
const [owner] = await db
843+
.select({ userId: account.userId })
844+
.from(account)
845+
.where(eq(account.id, resolved.accountId))
846+
.limit(1)
847+
ownerUserId = owner?.userId
848+
}
849+
if (!ownerUserId) {
850+
return undefined
851+
}
852+
return (await refreshAccessTokenIfNeeded(credentialId, ownerUserId, requestId)) ?? undefined
853+
}
854+
811855
export const slackHandler: WebhookProviderHandler = {
812856
verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) {
813857
const signingSecret = providerConfig.signingSecret as string | undefined
@@ -870,34 +914,19 @@ export const slackHandler: WebhookProviderHandler = {
870914
* `actions[]` and no Events-API `event` envelope), and the Events API
871915
* (app_mention, message, reaction_added, ... nested under `event`).
872916
*/
873-
async formatInput({ body, webhook, requestId }: FormatInputContext): Promise<FormatInputResult> {
917+
async formatInput({
918+
body,
919+
webhook,
920+
requestId,
921+
credentialOwnerUserId,
922+
}: FormatInputContext): Promise<FormatInputResult> {
874923
const b = isRecordLike(body) ? body : {}
875924
const providerConfig = (webhook.providerConfig as Record<string, unknown>) || {}
876-
let botToken = providerConfig.botToken as string | undefined
877-
// Reusable custom Slack bot credential: use its stored bot token directly.
878-
if (!botToken && typeof providerConfig.credentialId === 'string') {
879-
const botCredential = await getSlackBotCredential(providerConfig.credentialId)
880-
if (botCredential) botToken = botCredential.botToken
881-
}
882-
// Native (slack_app) triggers carry an OAuth credential rather than a pasted
883-
// bot token; resolve it via the credential's OWNER (not the execution actor
884-
// in workflow.userId, who may not own the credential) so reaction-message
885-
// text and file downloads work.
886-
if (!botToken && typeof providerConfig.credentialId === 'string') {
887-
const credentialId = providerConfig.credentialId
888-
const resolved = await resolveOAuthAccountId(credentialId)
889-
if (resolved?.accountId) {
890-
const [owner] = await db
891-
.select({ userId: account.userId })
892-
.from(account)
893-
.where(eq(account.id, resolved.accountId))
894-
.limit(1)
895-
if (owner?.userId) {
896-
botToken =
897-
(await refreshAccessTokenIfNeeded(credentialId, owner.userId, requestId)) ?? undefined
898-
}
899-
}
900-
}
925+
const botToken = await resolveSlackWebhookBotToken(
926+
providerConfig,
927+
requestId,
928+
credentialOwnerUserId
929+
)
901930
const includeFiles = Boolean(providerConfig.includeFiles)
902931

903932
if (typeof b?.command === 'string' && b.command.startsWith('/')) {

apps/sim/lib/webhooks/providers/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ export interface FormatInputContext {
3838
/** HTTP method of the delivering request. Empty on legacy queued jobs. */
3939
method: string
4040
requestId: string
41+
/**
42+
* Owner of the webhook's OAuth credential, when the worker already resolved
43+
* it for execution metadata. Lets a provider skip re-resolving the same
44+
* credential → account → owner chain; absent on paths that never resolved it.
45+
*/
46+
credentialOwnerUserId?: string
4147
}
4248

4349
/** Result of custom input preparation. */

0 commit comments

Comments
 (0)