Skip to content

Commit db63eba

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(quickbooks): enforce auth and record invariants
1 parent 78b2768 commit db63eba

6 files changed

Lines changed: 111 additions & 16 deletions

File tree

apps/sim/app/api/tools/quickbooks/upload-attachment/route.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,28 @@ describe('POST /api/tools/quickbooks/upload-attachment', () => {
247247
})
248248
})
249249

250+
it('rejects QuickBooks upload responses with an empty attachment object', async () => {
251+
mockFetch.mockResolvedValueOnce(
252+
new Response(
253+
JSON.stringify({
254+
AttachableResponse: [{ Attachable: {} }],
255+
time: '2026-07-29T23:00:00Z',
256+
}),
257+
{
258+
headers: { 'content-type': 'application/json' },
259+
status: 200,
260+
}
261+
)
262+
)
263+
264+
const response = await POST(createMockRequest('POST', baseBody))
265+
expect(response.status).toBe(500)
266+
await expect(response.json()).resolves.toEqual({
267+
success: false,
268+
error: 'QuickBooks attachment upload returned no attachment',
269+
})
270+
})
271+
250272
it('rejects files over the buffered 25 MB attachment limit', async () => {
251273
mockProcessFilesToUserFiles.mockReturnValueOnce([
252274
{ ...baseBody.file, size: 25 * 1024 * 1024 + 1 },
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
const QUICKBOOKS_MAX_ACCESS_TOKEN_LENGTH = 4096
2+
3+
export function normalizeQuickBooksAccessToken(accessToken: string): string {
4+
if (/[\r\n]/.test(accessToken)) {
5+
throw new Error('QuickBooks access token contains invalid characters')
6+
}
7+
if (accessToken.length > QUICKBOOKS_MAX_ACCESS_TOKEN_LENGTH) {
8+
throw new Error(
9+
`QuickBooks access token must be ${QUICKBOOKS_MAX_ACCESS_TOKEN_LENGTH} characters or less`
10+
)
11+
}
12+
13+
const normalizedAccessToken = accessToken.trim()
14+
if (!normalizedAccessToken) {
15+
throw new Error('QuickBooks access token is required')
16+
}
17+
return normalizedAccessToken
18+
}

apps/sim/lib/oauth/quickbooks.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,35 @@ describe('fetchQuickBooksUserInfo', () => {
5656
)
5757
})
5858

59+
it('normalizes and validates access tokens before making a user-info request', async () => {
60+
const fetchMock = vi.fn().mockResolvedValue(
61+
new Response(
62+
JSON.stringify({
63+
sub: 'intuit-user-1',
64+
})
65+
)
66+
)
67+
vi.stubGlobal('fetch', fetchMock)
68+
69+
await expect(fetchQuickBooksUserInfo(' access-token ', true)).resolves.toMatchObject({
70+
sub: 'intuit-user-1',
71+
})
72+
expect(fetchMock).toHaveBeenCalledWith(
73+
'https://sandbox-accounts.platform.intuit.com/v1/openid_connect/userinfo',
74+
expect.objectContaining({
75+
headers: expect.objectContaining({ Authorization: 'Bearer access-token' }),
76+
})
77+
)
78+
79+
await expect(fetchQuickBooksUserInfo('token\r\nX-Injected: true', true)).rejects.toThrow(
80+
'QuickBooks access token contains invalid characters'
81+
)
82+
await expect(fetchQuickBooksUserInfo('x'.repeat(4097), true)).rejects.toThrow(
83+
'QuickBooks access token must be 4096 characters or less'
84+
)
85+
expect(fetchMock).toHaveBeenCalledOnce()
86+
})
87+
5988
it('falls back to production when the sandbox endpoint rejects the token', async () => {
6089
const fetchMock = vi
6190
.fn()

apps/sim/lib/oauth/quickbooks.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
readResponseJsonWithLimit,
55
readResponseTextWithLimit,
66
} from '@/lib/core/utils/stream-limits'
7+
import { normalizeQuickBooksAccessToken } from '@/lib/oauth/quickbooks-token'
78

89
const logger = createLogger('QuickBooksOAuth')
910

@@ -38,6 +39,7 @@ export async function fetchQuickBooksUserInfo(
3839
if (!accessToken) {
3940
throw new Error('QuickBooks OAuth token response did not include an access token')
4041
}
42+
const normalizedAccessToken = normalizeQuickBooksAccessToken(accessToken)
4143

4244
const failures: string[] = []
4345

@@ -46,7 +48,7 @@ export async function fetchQuickBooksUserInfo(
4648
const response = await fetch(endpoint, {
4749
headers: {
4850
Accept: 'application/json',
49-
Authorization: `Bearer ${accessToken}`,
51+
Authorization: `Bearer ${normalizedAccessToken}`,
5052
},
5153
})
5254

apps/sim/tools/quickbooks/generic_operations.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,32 @@ describe('QuickBooks generic operations', () => {
592592
).rejects.toThrow('QuickBooks API response did not include ExchangeRate')
593593
})
594594

595+
it('rejects successful entity responses without a record ID', async () => {
596+
await expect(
597+
quickBooksCreateRecordTool.transformResponse?.(
598+
new Response(JSON.stringify({ Vendor: {} }), { status: 200 }),
599+
{
600+
accessToken: 'token',
601+
realmId: '123145',
602+
entity: 'Vendor',
603+
payload: { DisplayName: 'Acme Supplies' },
604+
}
605+
)
606+
).rejects.toThrow('QuickBooks API response did not include Vendor')
607+
608+
await expect(
609+
quickBooksCreateRecordTool.transformResponse?.(
610+
new Response(JSON.stringify({ Vendor: { DisplayName: 'Acme Supplies' } }), { status: 200 }),
611+
{
612+
accessToken: 'token',
613+
realmId: '123145',
614+
entity: 'Vendor',
615+
payload: { DisplayName: 'Acme Supplies' },
616+
}
617+
)
618+
).rejects.toThrow('QuickBooks API response did not include Vendor')
619+
})
620+
595621
it('transforms QuickBooks report sections', async () => {
596622
const response = new Response(
597623
JSON.stringify({

apps/sim/tools/quickbooks/utils.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { truncate } from '@sim/utils/string'
22
import { readResponseTextWithLimit } from '@/lib/core/utils/stream-limits'
3+
import { normalizeQuickBooksAccessToken } from '@/lib/oauth/quickbooks-token'
34
import type {
45
QuickBooksApiEnvelope,
56
QuickBooksAttachmentEntityName,
@@ -80,20 +81,8 @@ const QUICKBOOKS_RESOURCE_NAMES: Record<QuickBooksEntityName, string> = {
8081
}
8182

8283
export function buildQuickBooksHeaders(accessToken: string): Record<string, string> {
83-
if (/[\r\n]/.test(accessToken)) {
84-
throw new Error('QuickBooks access token contains invalid characters')
85-
}
86-
if (accessToken.length > 4096) {
87-
throw new Error('QuickBooks access token must be 4096 characters or less')
88-
}
89-
90-
const normalizedAccessToken = accessToken.trim()
91-
if (!normalizedAccessToken) {
92-
throw new Error('QuickBooks access token is required')
93-
}
94-
9584
return {
96-
Authorization: `Bearer ${normalizedAccessToken}`,
85+
Authorization: `Bearer ${normalizeQuickBooksAccessToken(accessToken)}`,
9786
Accept: 'application/json',
9887
'Content-Type': 'application/json',
9988
}
@@ -500,7 +489,7 @@ export async function parseQuickBooksJson(response: Response): Promise<QuickBook
500489
export function assertQuickBooksAttachmentUploadResponse(
501490
data: QuickBooksApiEnvelope
502491
): QuickBooksApiEnvelope {
503-
const uploaded = data.AttachableResponse?.some((item) => isQuickBooksRecord(item.Attachable))
492+
const uploaded = data.AttachableResponse?.some((item) => hasQuickBooksRecordId(item.Attachable))
504493
if (!uploaded) {
505494
throw new Error('QuickBooks attachment upload returned no attachment')
506495
}
@@ -555,7 +544,8 @@ export function extractQuickBooksRecord(
555544
entity: string
556545
): QuickBooksRecord {
557546
const record = data[entity]
558-
if (!isQuickBooksRecord(record)) {
547+
const requiresId = entity !== 'Preferences' && entity !== 'ExchangeRate'
548+
if (!isNonEmptyQuickBooksRecord(record) || (requiresId && !hasQuickBooksRecordId(record))) {
559549
throw new Error(`QuickBooks API response did not include ${entity}`)
560550
}
561551
return record
@@ -763,6 +753,14 @@ function isQuickBooksRecord(value: unknown): value is QuickBooksRecord {
763753
return value != null && typeof value === 'object' && !Array.isArray(value)
764754
}
765755

756+
function isNonEmptyQuickBooksRecord(value: unknown): value is QuickBooksRecord {
757+
return isQuickBooksRecord(value) && Object.keys(value).length > 0
758+
}
759+
760+
function hasQuickBooksRecordId(value: unknown): value is QuickBooksRecord {
761+
return isQuickBooksRecord(value) && typeof value.Id === 'string' && value.Id.trim().length > 0
762+
}
763+
766764
function extractQuickBooksError(data: QuickBooksApiEnvelope): string | null {
767765
const errors = [
768766
...extractFaultErrors(data.Fault),

0 commit comments

Comments
 (0)