Skip to content

Commit d51a1d6

Browse files
waleedlatif1waleedlatif1
committed
Merge branch 'fix/qb-webhook-hardening' into integration/quickbooks-doc-alignment
2 parents a0f38db + aa22bdd commit d51a1d6

9 files changed

Lines changed: 245 additions & 46 deletions

File tree

‎apps/sim/app/api/webhooks/quickbooks/[appKey]/route.test.ts‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ vi.mock('@/lib/core/admission/gate', () => ({
2020
tryAdmit: vi.fn(() => ({ release: mockRelease })),
2121
}))
2222
vi.mock('@/lib/webhooks/quickbooks-credentials', () => ({
23-
getQuickBooksWebhookVerifierTokensByAppKey: mockVerifierTokens,
23+
streamQuickBooksWebhookVerifierTokensByAppKey: mockVerifierTokens,
2424
}))
2525
vi.mock('@/lib/core/utils/with-route-handler', () => ({
2626
withRouteHandler:
@@ -68,10 +68,16 @@ function callPost(webhookRequest: NextRequest, appKey = APP_KEY): Promise<Respon
6868
return POST(webhookRequest, { params: Promise.resolve({ appKey }) })
6969
}
7070

71+
function mockTokens(...tokens: string[]): void {
72+
mockVerifierTokens.mockImplementation(async function* (): AsyncGenerator<string> {
73+
yield* tokens
74+
})
75+
}
76+
7177
describe('QuickBooks webhook ingress route', () => {
7278
beforeEach(() => {
7379
vi.clearAllMocks()
74-
mockVerifierTokens.mockResolvedValue(['verifier'])
80+
mockTokens('verifier')
7581
requestUtilsMockFns.mockGenerateRequestId.mockReturnValue('request-1')
7682
mockEnqueue.mockResolvedValue('job-1')
7783
})
@@ -97,19 +103,33 @@ describe('QuickBooks webhook ingress route', () => {
97103
})
98104

99105
it('accepts any verifier token configured by a connection for the same Intuit app', async () => {
100-
mockVerifierTokens.mockResolvedValue(['stale-verifier', 'current-verifier'])
106+
mockTokens('stale-verifier', 'current-verifier')
101107

102108
expect((await callPost(signedRequest([validEvent], 'current-verifier'))).status).toBe(200)
103109
})
104110

105111
it('fails closed for unknown app keys and missing signatures', async () => {
106112
expect((await callPost(signedRequest([validEvent]), 'invalid')).status).toBe(404)
107-
mockVerifierTokens.mockResolvedValueOnce([])
108-
expect((await callPost(signedRequest([validEvent]))).status).toBe(404)
113+
mockTokens()
114+
expect((await callPost(signedRequest([validEvent]))).status).toBe(401)
115+
mockTokens('verifier')
109116
expect((await callPost(request(JSON.stringify([validEvent])))).status).toBe(401)
110117
expect(mockEnqueue).not.toHaveBeenCalled()
111118
})
112119

120+
it('acknowledges a batch that carries an unmodelled event instead of stalling the app queue', async () => {
121+
const unmodelledEvent = { ...validEvent, id: 'event-2', type: undefined }
122+
const response = await callPost(signedRequest([validEvent, unmodelledEvent]))
123+
124+
expect(response.status).toBe(200)
125+
expect(mockEnqueue).toHaveBeenCalledWith(expect.objectContaining({ events: [validEvent] }))
126+
})
127+
128+
it('acknowledges a batch whose events are all unmodelled without enqueueing', async () => {
129+
expect((await callPost(signedRequest([{ id: 'event-1' }]))).status).toBe(200)
130+
expect(mockEnqueue).not.toHaveBeenCalled()
131+
})
132+
113133
it('rejects malformed signed payloads and batches over the event bound', async () => {
114134
expect((await callPost(signedRequest({ invalid: true }))).status).toBe(400)
115135
const events = Array.from({ length: 1001 }, (_, index) => ({

‎apps/sim/app/api/webhooks/quickbooks/[appKey]/route.ts‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import {
5-
quickBooksWebhookEventsSchema,
5+
type QuickBooksWebhookEvent,
6+
quickBooksWebhookEventSchema,
67
quickBooksWebhookParamsSchema,
78
} from '@/lib/api/contracts/webhooks'
89
import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate'
@@ -14,15 +15,17 @@ import {
1415
} from '@/lib/core/utils/stream-limits'
1516
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1617
import { WEBHOOK_MAX_BODY_BYTES } from '@/lib/webhooks/constants'
17-
import { verifyQuickBooksSignatureAgainstVerifierTokens } from '@/lib/webhooks/providers/quickbooks'
18-
import { getQuickBooksWebhookVerifierTokensByAppKey } from '@/lib/webhooks/quickbooks-credentials'
18+
import { verifyQuickBooksSignatureAgainstVerifierTokenStream } from '@/lib/webhooks/providers/quickbooks'
19+
import { streamQuickBooksWebhookVerifierTokensByAppKey } from '@/lib/webhooks/quickbooks-credentials'
1920
import {
2021
enqueueQuickBooksWebhookIngress,
2122
type QuickBooksWebhookIngressPayload,
2223
} from '@/background/quickbooks-webhook-ingress'
2324

2425
const logger = createLogger('QuickBooksWebhookIngress')
2526
const BODY_LABEL = 'QuickBooks webhook body'
27+
/** Mirrors the batch ceiling `quickBooksWebhookEventsSchema` declares for the same envelope. */
28+
const MAX_WEBHOOK_EVENTS = 1000
2629

2730
export const dynamic = 'force-dynamic'
2831
export const runtime = 'nodejs'
@@ -62,14 +65,10 @@ export const POST = withRouteHandler(
6265
throw error
6366
}
6467

65-
const verifierTokens = await getQuickBooksWebhookVerifierTokensByAppKey(appKey)
66-
if (verifierTokens.length === 0) {
67-
return NextResponse.json({ error: 'Webhook not found' }, { status: 404 })
68-
}
69-
const authError = verifyQuickBooksSignatureAgainstVerifierTokens(
68+
const authError = await verifyQuickBooksSignatureAgainstVerifierTokenStream(
7069
rawBody,
7170
request.headers.get('intuit-signature'),
72-
verifierTokens,
71+
streamQuickBooksWebhookVerifierTokensByAppKey(appKey),
7372
requestId
7473
)
7574
if (authError) return authError
@@ -80,17 +79,29 @@ export const POST = withRouteHandler(
8079
} catch {
8180
return NextResponse.json({ error: 'Invalid JSON' }, { status: 400 })
8281
}
83-
const parsed = quickBooksWebhookEventsSchema.safeParse(json)
84-
if (!parsed.success) {
85-
logger.warn(`[${requestId}] Invalid QuickBooks webhook envelope`, {
86-
issues: parsed.error.issues,
87-
})
82+
if (!Array.isArray(json) || json.length === 0 || json.length > MAX_WEBHOOK_EVENTS) {
83+
logger.warn(`[${requestId}] Invalid QuickBooks webhook envelope`)
8884
return NextResponse.json({ error: 'Invalid webhook envelope' }, { status: 400 })
8985
}
9086

87+
const events: QuickBooksWebhookEvent[] = []
88+
let droppedCount = 0
89+
for (const entry of json) {
90+
const parsedEvent = quickBooksWebhookEventSchema.safeParse(entry)
91+
if (parsedEvent.success) events.push(parsedEvent.data)
92+
else droppedCount += 1
93+
}
94+
if (droppedCount > 0) {
95+
logger.warn(`[${requestId}] Dropped unmodelled QuickBooks webhook events`, {
96+
droppedCount,
97+
eventCount: json.length,
98+
})
99+
}
100+
if (events.length === 0) return NextResponse.json({ ok: true })
101+
91102
const payload: QuickBooksWebhookIngressPayload = {
92103
appKey,
93-
events: parsed.data,
104+
events,
94105
headers: {
95106
'content-type': request.headers.get('content-type') ?? 'application/json',
96107
},
@@ -99,7 +110,7 @@ export const POST = withRouteHandler(
99110
}
100111
const jobId = await enqueueQuickBooksWebhookIngress(payload)
101112
logger.info(`[${requestId}] Accepted QuickBooks webhook delivery`, {
102-
eventCount: parsed.data.length,
113+
eventCount: events.length,
103114
jobId,
104115
})
105116
return NextResponse.json({ ok: true })

‎apps/sim/background/quickbooks-webhook-ingress.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,27 @@ describe('QuickBooks webhook ingress job', () => {
125125
expect(mockEnqueue).toHaveBeenCalledOnce()
126126
})
127127

128+
it('ignores an event whose company identity can never be routed', async () => {
129+
mockFindWebhooks.mockResolvedValue([])
130+
const unroutablePayload: QuickBooksWebhookIngressPayload = {
131+
...payload,
132+
events: [{ ...event, intuitaccountid: 'not-a-realm' }, payload.events[1]],
133+
}
134+
135+
await expect(executeQuickBooksWebhookIngress(unroutablePayload)).resolves.toEqual({
136+
failed: 0,
137+
ignored: 1,
138+
processed: 0,
139+
targetCount: 0,
140+
})
141+
expect(mockFindWebhooks).toHaveBeenCalledOnce()
142+
expect(mockFindWebhooks).toHaveBeenCalledWith(
143+
`${payload.appKey}:789`,
144+
'request-1',
145+
'quickbooks'
146+
)
147+
})
148+
128149
it('continues later events when targets cannot be resolved', async () => {
129150
mockFindWebhooks
130151
.mockRejectedValueOnce(new Error('database unavailable'))

‎apps/sim/background/quickbooks-webhook-ingress.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createHash } from 'node:crypto'
22
import { createLogger } from '@sim/logger'
3+
import { getErrorMessage } from '@sim/utils/errors'
34
import { task } from '@trigger.dev/sdk'
45
import { NextRequest } from 'next/server'
56
import type { QuickBooksWebhookEvent } from '@/lib/api/contracts/webhooks'
@@ -37,6 +38,19 @@ export async function executeQuickBooksWebhookIngress(
3738
let targetCount = 0
3839

3940
for (const [eventIndex, event] of payload.events.entries()) {
41+
let routingKey: string
42+
try {
43+
routingKey = buildQuickBooksWebhookRoutingKey(payload.appKey, event.intuitaccountid)
44+
} catch (error) {
45+
ignored += 1
46+
logger.warn(`[${payload.requestId}] QuickBooks webhook event is not routable`, {
47+
error: getErrorMessage(error, 'Unknown error'),
48+
eventId: event.id,
49+
eventIndex,
50+
})
51+
continue
52+
}
53+
4054
const request = new NextRequest(
4155
`http://internal/api/webhooks/quickbooks/${encodeURIComponent(payload.appKey)}`,
4256
{
@@ -47,7 +61,6 @@ export async function executeQuickBooksWebhookIngress(
4761
)
4862

4963
try {
50-
const routingKey = buildQuickBooksWebhookRoutingKey(payload.appKey, event.intuitaccountid)
5164
const targets = await findWebhooksByRoutingKey(routingKey, payload.requestId, 'quickbooks')
5265
targetCount += targets.length
5366

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

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest'
44
import {
55
quickBooksHandler,
66
verifyQuickBooksSignature,
7+
verifyQuickBooksSignatureAgainstVerifierTokenStream,
78
verifyQuickBooksSignatureAgainstVerifierTokens,
89
} from '@/lib/webhooks/providers/quickbooks'
910
import {
@@ -48,8 +49,72 @@ describe('QuickBooks webhook provider', () => {
4849
expect(isQuickBooksEventMatch('quickbooks_bill_events', event.type, ['updated'])).toBe(false)
4950
})
5051

52+
it('stops decrypting verifier tokens once one matches the signature', async () => {
53+
const body = JSON.stringify([event])
54+
const signature = crypto.createHmac('sha256', 'first-verifier').update(body).digest('base64')
55+
const yielded: string[] = []
56+
async function* tokens(): AsyncGenerator<string> {
57+
for (const token of ['first-verifier', 'second-verifier']) {
58+
yielded.push(token)
59+
yield token
60+
}
61+
}
62+
63+
expect(
64+
await verifyQuickBooksSignatureAgainstVerifierTokenStream(
65+
body,
66+
signature,
67+
tokens(),
68+
'request-stream-1'
69+
)
70+
).toBeNull()
71+
expect(yielded).toEqual(['first-verifier'])
72+
})
73+
74+
it('fails closed when no streamed verifier token matches', async () => {
75+
const body = JSON.stringify([event])
76+
async function* tokens(): AsyncGenerator<string> {
77+
yield 'first-verifier'
78+
}
79+
async function* noTokens(): AsyncGenerator<string> {}
80+
81+
expect(
82+
(
83+
await verifyQuickBooksSignatureAgainstVerifierTokenStream(
84+
body,
85+
'invalid',
86+
tokens(),
87+
'request-stream-2'
88+
)
89+
)?.status
90+
).toBe(401)
91+
expect(
92+
(
93+
await verifyQuickBooksSignatureAgainstVerifierTokenStream(
94+
body,
95+
'irrelevant',
96+
noTokens(),
97+
'request-stream-3'
98+
)
99+
)?.status
100+
).toBe(401)
101+
expect(
102+
(
103+
await verifyQuickBooksSignatureAgainstVerifierTokenStream(
104+
body,
105+
null,
106+
tokens(),
107+
'request-stream-4'
108+
)
109+
)?.status
110+
).toBe(401)
111+
})
112+
51113
it('normalizes Intuit void events to the configured voided action', async () => {
52-
for (const entity of ['invoice', 'payment']) {
114+
for (const [entity, entityType] of [
115+
['invoice', 'Invoice'],
116+
['payment', 'Payment'],
117+
]) {
53118
const voidEvent = { ...event, type: `qbo.${entity}.void.v1` }
54119
expect(
55120
isQuickBooksEventMatch(`quickbooks_${entity}_events`, voidEvent.type, ['voided'])
@@ -64,7 +129,7 @@ describe('QuickBooks webhook provider', () => {
64129
})
65130
expect(result.input).toMatchObject({
66131
eventType: `qbo.${entity}.void.v1`,
67-
entityType: entity,
132+
entityType,
68133
action: 'voided',
69134
})
70135
}
@@ -81,7 +146,7 @@ describe('QuickBooks webhook provider', () => {
81146
expect(result.input).toEqual({
82147
eventId: 'event-1',
83148
eventType: 'qbo.invoice.updated.v1',
84-
entityType: 'invoice',
149+
entityType: 'Invoice',
85150
action: 'updated',
86151
entityId: '123',
87152
realmId: '456',

0 commit comments

Comments
 (0)