Skip to content

Commit 73d69fe

Browse files
committed
fix(security): add try-catch to Webflow verifyAuth and clean up debug artifacts
- webflow: wrap HMAC computation and safeCompare in try-catch to return a clean 401 instead of an unhandled 500 when secretKey is a non-string truthy value (mirrors the defensive pattern already used by Airtable and HubSpot in the same PR) - airtable: remove CRITICAL_TRACE/TRACE/DEBUG log prefixes, redundant "Error logging handled by logging session" comments, and other stale implementation notes that pre-date the refactor
1 parent 49b9503 commit 73d69fe

2 files changed

Lines changed: 21 additions & 27 deletions

File tree

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

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,11 @@ async function fetchAndProcessAirtablePayloads(
5656
workflowData: Record<string, unknown>,
5757
requestId: string // Original request ID from the ping, used for the final execution log
5858
) {
59-
// Logging handles all error logging
6059
let currentCursor: number | null = null
6160
let mightHaveMore = true
6261
let payloadsFetched = 0
6362
let apiCallCount = 0
64-
// Use a Map to consolidate changes per record ID
6563
const consolidatedChangesMap = new Map<string, AirtableChange>()
66-
// Capture raw payloads from Airtable for exposure to workflows
6764
const allPayloads = []
6865
const localProviderConfig = {
6966
...((webhookData.providerConfig as Record<string, unknown>) || {}),
@@ -221,7 +218,6 @@ async function fetchAndProcessAirtablePayloads(
221218
error: errorMessage,
222219
}
223220
)
224-
// Error logging handled by logging session
225221
mightHaveMore = false
226222
break
227223
}
@@ -304,7 +300,6 @@ async function fetchAndProcessAirtablePayloads(
304300
}
305301
}
306302
}
307-
// TODO: Handle deleted records (`destroyedRecordIds`) if needed
308303
}
309304
}
310305
}
@@ -316,7 +311,6 @@ async function fetchAndProcessAirtablePayloads(
316311
if (nextCursor && typeof nextCursor === 'number' && nextCursor !== currentCursor) {
317312
currentCursor = nextCursor
318313

319-
// Follow exactly the old implementation - use awaited update instead of parallel
320314
const updatedConfig = {
321315
...localProviderConfig,
322316
externalWebhookCursor: currentCursor,
@@ -326,7 +320,7 @@ async function fetchAndProcessAirtablePayloads(
326320
await db
327321
.update(webhook)
328322
.set({
329-
providerConfig: updatedConfig, // Use full object
323+
providerConfig: updatedConfig,
330324
updatedAt: new Date(),
331325
})
332326
.where(eq(webhook.id, webhookData.id as string))
@@ -339,7 +333,6 @@ async function fetchAndProcessAirtablePayloads(
339333
cursor: currentCursor,
340334
error: err.message,
341335
})
342-
// Error logging handled by logging session
343336
mightHaveMore = false
344337
throw new Error('Failed to save Airtable cursor, stopping processing.') // Re-throw to break loop clearly
345338
}
@@ -358,7 +351,6 @@ async function fetchAndProcessAirtablePayloads(
358351
`[${requestId}] Network error calling Airtable GET /payloads (Call ${apiCallCount}) for webhook ${webhookData.id}`,
359352
fetchError
360353
)
361-
// Error logging handled by logging session
362354
mightHaveMore = false
363355
break
364356
}
@@ -371,7 +363,6 @@ async function fetchAndProcessAirtablePayloads(
371363

372364
if (finalConsolidatedChanges.length > 0 || allPayloads.length > 0) {
373365
try {
374-
// Build input exposing raw payloads and consolidated changes
375366
const latestPayload = allPayloads.length > 0 ? allPayloads[allPayloads.length - 1] : null
376367
const input: Record<string, unknown> = {
377368
payloads: allPayloads,
@@ -388,9 +379,8 @@ async function fetchAndProcessAirtablePayloads(
388379
},
389380
}
390381

391-
// CRITICAL EXECUTION TRACE POINT
392382
logger.info(
393-
`[${requestId}] CRITICAL_TRACE: Beginning workflow execution with ${finalConsolidatedChanges.length} Airtable changes`,
383+
`[${requestId}] Beginning workflow execution with ${finalConsolidatedChanges.length} Airtable changes`,
394384
{
395385
workflowId: workflowData.id,
396386
recordCount: finalConsolidatedChanges.length,
@@ -399,8 +389,7 @@ async function fetchAndProcessAirtablePayloads(
399389
}
400390
)
401391

402-
// Return the processed input for the trigger.dev task to handle
403-
logger.info(`[${requestId}] CRITICAL_TRACE: Airtable changes processed, returning input`, {
392+
logger.info(`[${requestId}] Airtable changes processed, returning input`, {
404393
workflowId: workflowData.id,
405394
recordCount: finalConsolidatedChanges.length,
406395
rawPayloadCount: allPayloads.length,
@@ -410,7 +399,7 @@ async function fetchAndProcessAirtablePayloads(
410399
return input
411400
} catch (processingError: unknown) {
412401
const err = processingError as Error
413-
logger.error(`[${requestId}] CRITICAL_TRACE: Error processing Airtable changes`, {
402+
logger.error(`[${requestId}] Error processing Airtable changes`, {
414403
workflowId: workflowData.id,
415404
error: err.message,
416405
stack: err.stack,
@@ -420,8 +409,7 @@ async function fetchAndProcessAirtablePayloads(
420409
throw processingError
421410
}
422411
} else {
423-
// DEBUG: Log when no changes are found
424-
logger.info(`[${requestId}] TRACE: No Airtable changes to process`, {
412+
logger.info(`[${requestId}] No Airtable changes to process`, {
425413
workflowId: workflowData.id,
426414
apiCallCount,
427415
webhookId: webhookData.id,
@@ -437,7 +425,6 @@ async function fetchAndProcessAirtablePayloads(
437425
error: (error as Error).message,
438426
}
439427
)
440-
// Error logging handled by logging session
441428
}
442429
}
443430

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,22 @@ export const webflowHandler: WebhookProviderHandler = {
4747
return new NextResponse('Unauthorized - Webhook timestamp expired', { status: 401 })
4848
}
4949

50-
// HMAC-SHA256 of "${timestamp}:${rawBody}"
51-
const computedHash = crypto
52-
.createHmac('sha256', secretKey)
53-
.update(`${timestamp}:${rawBody}`, 'utf8')
54-
.digest('hex')
55-
56-
if (!safeCompare(computedHash, signature)) {
57-
logger.warn(`[${requestId}] Webflow signature verification failed`)
58-
return new NextResponse('Unauthorized - Invalid Webflow signature', { status: 401 })
50+
try {
51+
// HMAC-SHA256 of "${timestamp}:${rawBody}"
52+
const computedHash = crypto
53+
.createHmac('sha256', secretKey)
54+
.update(`${timestamp}:${rawBody}`, 'utf8')
55+
.digest('hex')
56+
57+
if (!safeCompare(computedHash, signature)) {
58+
logger.warn(`[${requestId}] Webflow signature verification failed`)
59+
return new NextResponse('Unauthorized - Invalid Webflow signature', { status: 401 })
60+
}
61+
} catch (error) {
62+
logger.error(`[${requestId}] Error verifying Webflow signature`, {
63+
error: (error as Error).message,
64+
})
65+
return new NextResponse('Unauthorized - Signature verification error', { status: 401 })
5966
}
6067

6168
return null

0 commit comments

Comments
 (0)