Skip to content

Commit 5726709

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(quickbooks): store downloaded documents safely
1 parent b35dfd5 commit 5726709

11 files changed

Lines changed: 326 additions & 47 deletions

File tree

apps/docs/content/docs/en/integrations/quickbooks.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Sandbox credentials call only Intuit's sandbox API and are suitable for disposab
2121

2222
Run Financial Report exposes verified financial statements, aging, balance, sales, and expense reports while preserving QuickBooks' native columns and nested rows. Advanced controls appear only where QuickBooks supports them. Use Read Master Data to discover customer, vendor, account, item, class, and department IDs for report filters. Intuit recommends report periods of six months or less for performance, though Sim does not forbid longer accounting periods.
2323

24-
Attachments are not exposed by this version of the block.
24+
Document actions can read attachment metadata, add one File or Note attachment, download an attachment file, and download supported transactions as PDFs. Downloaded files are stored as Sim files for downstream blocks. Attachment deletion, bulk upload/download, and bulk email remain outside this version of the block.
2525
{/* MANUAL-CONTENT-END */}
2626

2727

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4949
)
5050
if (!parsed.success) return parsed.response
5151
const data = parsed.data.body
52+
request.signal.throwIfAborted()
5253
const url = buildQuickBooksCompanyUrl(
5354
data.realmId,
5455
data.attachmentKind === 'file' ? 'upload' : 'attachable'
@@ -66,8 +67,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6667
'Content-Type': 'application/json',
6768
},
6869
body: JSON.stringify(metadata),
70+
signal: request.signal,
6971
})
7072
} else {
73+
request.signal.throwIfAborted()
7174
const rawFile = assertSingleQuickBooksFile(data.file ?? undefined)
7275
const files = processFilesToUserFiles([rawFile], requestId, logger)
7376
if (files.length !== 1) throw new Error('Exactly one valid file is required')
@@ -79,12 +82,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7982
try {
8083
downloaded = await downloadServableFileFromStorage(file, requestId, logger, {
8184
maxBytes: MAX_FILE_SIZE,
85+
signal: request.signal,
8286
})
8387
} catch (error) {
8488
const notReady = docNotReadyResponse(error)
8589
if (notReady) return notReady
8690
throw error
8791
}
92+
request.signal.throwIfAborted()
8893
assertKnownSizeWithinLimit(
8994
downloaded.buffer.length,
9095
MAX_FILE_SIZE,
@@ -118,15 +123,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
118123
new Blob([new Uint8Array(downloaded.buffer)], { type: mimeType }),
119124
resolvedName
120125
)
126+
request.signal.throwIfAborted()
121127
response = await fetch(url, {
122128
method: 'POST',
123129
headers: buildQuickBooksHeaders(data.accessToken),
124130
body: formData,
131+
signal: request.signal,
125132
})
126133
}
127134

128-
if (!response.ok) throw await getQuickBooksDocumentError(response)
129-
const transformed = await parseQuickBooksAttachableResponse(response)
135+
if (!response.ok) throw await getQuickBooksDocumentError(response, request.signal)
136+
const transformed = await parseQuickBooksAttachableResponse(response, request.signal)
130137
return NextResponse.json({
131138
success: true,
132139
output: {

apps/sim/app/api/tools/quickbooks/documents.test.ts

Lines changed: 164 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,22 @@ import {
77
inputValidationMock,
88
inputValidationMockFns,
99
} from '@sim/testing'
10+
import { NextRequest } from 'next/server'
1011
import { beforeEach, describe, expect, it, vi } from 'vitest'
1112
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
1213

1314
const {
1415
mockAssertToolFileAccess,
1516
mockDownloadServableFileFromStorage,
1617
mockProcessFilesToUserFiles,
18+
mockUploadCopilotFile,
19+
mockUploadExecutionFile,
1720
} = vi.hoisted(() => ({
1821
mockAssertToolFileAccess: vi.fn(),
1922
mockDownloadServableFileFromStorage: vi.fn(),
2023
mockProcessFilesToUserFiles: vi.fn(),
24+
mockUploadCopilotFile: vi.fn(),
25+
mockUploadExecutionFile: vi.fn(),
2126
}))
2227

2328
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
@@ -30,6 +35,10 @@ vi.mock('@/lib/uploads/utils/file-utils', () => ({
3035
vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
3136
downloadServableFileFromStorage: mockDownloadServableFileFromStorage,
3237
}))
38+
vi.mock('@/lib/uploads/contexts/copilot', () => ({ uploadCopilotFile: mockUploadCopilotFile }))
39+
vi.mock('@/lib/uploads/contexts/execution', () => ({
40+
uploadExecutionFile: mockUploadExecutionFile,
41+
}))
3342

3443
import { POST as addAttachment } from '@/app/api/tools/quickbooks/add-attachment/route'
3544
import { POST as downloadAttachment } from '@/app/api/tools/quickbooks/download-attachment/route'
@@ -45,6 +54,15 @@ const attachmentFile = {
4554
type: 'application/pdf',
4655
}
4756

57+
function createAbortableRequest(body: unknown, signal: AbortSignal): NextRequest {
58+
return new NextRequest('http://localhost:3000/api/tools/quickbooks/test', {
59+
method: 'POST',
60+
headers: { 'Content-Type': 'application/json' },
61+
body: JSON.stringify(body),
62+
signal,
63+
})
64+
}
65+
4866
beforeEach(() => {
4967
vi.clearAllMocks()
5068
vi.stubGlobal('fetch', mockFetch)
@@ -59,6 +77,22 @@ beforeEach(() => {
5977
buffer: Buffer.from('%PDF-1.4 fixture'),
6078
contentType: 'application/pdf',
6179
})
80+
mockUploadCopilotFile.mockResolvedValue({
81+
id: 'copilot-file-1',
82+
key: 'copilot/user-1/receipt.pdf',
83+
name: 'receipt.pdf',
84+
size: 10,
85+
type: 'application/pdf',
86+
url: 'https://files.example/receipt.pdf',
87+
})
88+
mockUploadExecutionFile.mockResolvedValue({
89+
id: 'execution-file-1',
90+
key: 'execution/workspace-1/workflow-1/execution-1/invoice.pdf',
91+
name: 'invoice.pdf',
92+
size: 16,
93+
type: 'application/pdf',
94+
url: 'https://files.example/invoice.pdf',
95+
})
6296
mockValidateUrlWithDNS.mockResolvedValue({
6397
isValid: true,
6498
resolvedIP: '203.0.113.8',
@@ -92,13 +126,29 @@ describe('QuickBooks document API routes', () => {
92126
transactionType: 'invoice',
93127
transactionId: 'A/B',
94128
fileName: '../invoice.pdf',
129+
workspaceId: 'workspace-1',
130+
workflowId: 'workflow-1',
131+
executionId: 'execution-1',
95132
})
96133
)
97134
const body = await response.json()
98135

99136
expect(response.status).toBe(200)
100137
expect(body.output.fileName).toBe('invoice.pdf')
101-
expect(body.output.file.data).toBe(Buffer.from('%PDF-1.4 fixture').toString('base64'))
138+
expect(body.output.file).toMatchObject({
139+
key: 'execution/workspace-1/workflow-1/execution-1/invoice.pdf',
140+
name: 'invoice.pdf',
141+
})
142+
expect(body.output.file).not.toHaveProperty('data')
143+
expect(body.output.file).not.toHaveProperty('base64')
144+
expect(mockUploadExecutionFile).toHaveBeenCalledWith(
145+
{ workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-1' },
146+
expect.any(Buffer),
147+
'invoice.pdf',
148+
'application/pdf',
149+
'user-1'
150+
)
151+
expect(mockUploadCopilotFile).not.toHaveBeenCalled()
102152
expect(mockFetch).toHaveBeenCalledTimes(1)
103153
expect(String(mockFetch.mock.calls[0][0])).toContain('/invoice/A%2FB/pdf')
104154
})
@@ -175,6 +225,7 @@ describe('QuickBooks document API routes', () => {
175225
expect(body.output).toMatchObject({ attachmentId: '9', attachmentKind: 'note' })
176226
expect(body.output.attachment).toEqual({ Id: '9', Note: 'Audit note' })
177227
expect(mockFetch).toHaveBeenCalledTimes(1)
228+
expect(mockFetch.mock.calls[0][1].signal).toBeInstanceOf(AbortSignal)
178229
expect(JSON.parse(mockFetch.mock.calls[0][1].body)).toMatchObject({
179230
Note: 'Audit note',
180231
AttachableRef: [{ EntityRef: { type: 'Invoice', value: '77' } }],
@@ -210,7 +261,7 @@ describe('QuickBooks document API routes', () => {
210261
attachmentFile,
211262
expect.any(String),
212263
expect.anything(),
213-
{ maxBytes: MAX_FILE_SIZE }
264+
{ maxBytes: MAX_FILE_SIZE, signal: expect.any(AbortSignal) }
214265
)
215266
const formData = mockFetch.mock.calls[0][1].body as FormData
216267
expect(formData.get('file_metadata_01')).toBeInstanceOf(Blob)
@@ -343,6 +394,18 @@ describe('QuickBooks document API routes', () => {
343394
mimeType: 'application/pdf',
344395
size: 10,
345396
})
397+
expect(body.output.file).toMatchObject({
398+
key: 'copilot/user-1/receipt.pdf',
399+
name: 'receipt.pdf',
400+
})
401+
expect(body.output.file).not.toHaveProperty('data')
402+
expect(body.output.file).not.toHaveProperty('base64')
403+
expect(mockUploadCopilotFile).toHaveBeenCalledWith({
404+
buffer: expect.any(Buffer),
405+
fileName: 'receipt.pdf',
406+
contentType: 'application/pdf',
407+
userId: 'user-1',
408+
})
346409
expect(mockFetch).toHaveBeenCalledTimes(1)
347410
expect(mockValidateUrlWithDNS).toHaveBeenCalledWith(
348411
'https://intuit-download.example/receipt.pdf',
@@ -351,7 +414,12 @@ describe('QuickBooks document API routes', () => {
351414
expect(mockSecureFetchWithPinnedIP).toHaveBeenCalledWith(
352415
'https://intuit-download.example/receipt.pdf',
353416
'203.0.113.8',
354-
{ method: 'GET', maxResponseBytes: MAX_FILE_SIZE, stripAuthOnRedirect: true }
417+
{
418+
method: 'GET',
419+
maxResponseBytes: MAX_FILE_SIZE,
420+
stripAuthOnRedirect: true,
421+
signal: expect.any(AbortSignal),
422+
}
355423
)
356424
})
357425

@@ -436,4 +504,97 @@ describe('QuickBooks document API routes', () => {
436504
expect(body.error).toContain('tracking-1')
437505
expect(body.error).not.toContain('access-token')
438506
})
507+
508+
it('cancels PDF and attachment downloads before storing their bytes', async () => {
509+
const pdfController = new AbortController()
510+
const pdfRequest = createAbortableRequest(
511+
{ ...auth, transactionType: 'invoice', transactionId: '1' },
512+
pdfController.signal
513+
)
514+
mockFetch.mockImplementationOnce(async (_url, init) => {
515+
expect(init.signal).toBe(pdfRequest.signal)
516+
pdfController.abort()
517+
return new Response('%PDF-1.4 fixture', {
518+
headers: { 'content-type': 'application/pdf' },
519+
})
520+
})
521+
522+
const pdfResponse = await downloadTransactionPdf(pdfRequest)
523+
expect(pdfResponse.status).toBe(500)
524+
expect(mockUploadExecutionFile).not.toHaveBeenCalled()
525+
expect(mockUploadCopilotFile).not.toHaveBeenCalled()
526+
527+
vi.clearAllMocks()
528+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
529+
success: true,
530+
userId: 'user-1',
531+
authType: 'internal_jwt',
532+
})
533+
mockValidateUrlWithDNS.mockResolvedValue({
534+
isValid: true,
535+
resolvedIP: '203.0.113.8',
536+
originalHostname: 'intuit-download.example',
537+
})
538+
const attachmentController = new AbortController()
539+
const attachmentRequest = createAbortableRequest(
540+
{ ...auth, attachmentId: '15' },
541+
attachmentController.signal
542+
)
543+
mockFetch.mockResolvedValueOnce(new Response('https://intuit-download.example/receipt.pdf'))
544+
mockSecureFetchWithPinnedIP.mockImplementationOnce(async (_url, _ip, init) => {
545+
expect(init.signal).toBe(attachmentRequest.signal)
546+
attachmentController.abort()
547+
return new Response('file bytes', { headers: { 'content-type': 'application/pdf' } })
548+
})
549+
550+
const attachmentResponse = await downloadAttachment(attachmentRequest)
551+
expect(attachmentResponse.status).toBe(500)
552+
expect(mockUploadExecutionFile).not.toHaveBeenCalled()
553+
expect(mockUploadCopilotFile).not.toHaveBeenCalled()
554+
})
555+
556+
it('does not begin an attachment mutation when cancellation arrives after file loading', async () => {
557+
const controller = new AbortController()
558+
const request = createAbortableRequest(
559+
{
560+
...auth,
561+
attachmentKind: 'file',
562+
targetType: 'invoice',
563+
targetId: '1',
564+
file: attachmentFile,
565+
},
566+
controller.signal
567+
)
568+
mockDownloadServableFileFromStorage.mockImplementationOnce(
569+
async (_file, _id, _logger, opts) => {
570+
expect(opts.signal).toBe(request.signal)
571+
controller.abort()
572+
return { buffer: Buffer.from('%PDF-1.4 fixture'), contentType: 'application/pdf' }
573+
}
574+
)
575+
576+
const response = await addAttachment(request)
577+
expect(response.status).toBe(500)
578+
expect(mockFetch).not.toHaveBeenCalled()
579+
})
580+
581+
it('does not begin a note attachment mutation when the request is already cancelled', async () => {
582+
const controller = new AbortController()
583+
controller.abort()
584+
const response = await addAttachment(
585+
createAbortableRequest(
586+
{
587+
...auth,
588+
attachmentKind: 'note',
589+
targetType: 'invoice',
590+
targetId: '1',
591+
note: 'Audit note',
592+
},
593+
controller.signal
594+
)
595+
)
596+
597+
expect(response.status).toBe(500)
598+
expect(mockFetch).not.toHaveBeenCalled()
599+
})
439600
})

0 commit comments

Comments
 (0)