Skip to content

Commit af5a745

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(quickbooks): stop cancelled attachment downloads
1 parent 5726709 commit af5a745

4 files changed

Lines changed: 70 additions & 0 deletions

File tree

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,27 @@ describe('QuickBooks document API routes', () => {
553553
expect(mockUploadCopilotFile).not.toHaveBeenCalled()
554554
})
555555

556+
it('does not begin the pinned attachment fetch when cancellation arrives during DNS validation', async () => {
557+
const controller = new AbortController()
558+
const request = createAbortableRequest({ ...auth, attachmentId: '15' }, controller.signal)
559+
mockFetch.mockResolvedValueOnce(new Response('https://intuit-download.example/receipt.pdf'))
560+
mockValidateUrlWithDNS.mockImplementationOnce(async () => {
561+
controller.abort()
562+
return {
563+
isValid: true,
564+
resolvedIP: '203.0.113.8',
565+
originalHostname: 'intuit-download.example',
566+
}
567+
})
568+
569+
const response = await downloadAttachment(request)
570+
571+
expect(response.status).toBe(500)
572+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
573+
expect(mockUploadExecutionFile).not.toHaveBeenCalled()
574+
expect(mockUploadCopilotFile).not.toHaveBeenCalled()
575+
})
576+
556577
it('does not begin an attachment mutation when cancellation arrives after file loading', async () => {
557578
const controller = new AbortController()
558579
const request = createAbortableRequest(

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
9090
const validation = await validateUrlWithDNS(temporaryUrl, 'QuickBooks attachment URL')
9191
if (!validation.isValid || !validation.resolvedIP)
9292
throw new Error(validation.error || 'QuickBooks attachment URL is invalid')
93+
request.signal.throwIfAborted()
9394

9495
const downloadResponse = await secureFetchWithPinnedIP(temporaryUrl, validation.resolvedIP, {
9596
method: 'GET',

apps/sim/lib/core/security/input-validation.server.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,7 @@ export async function secureFetchWithPinnedIP(
966966
options: SecureFetchOptions & { allowHttp?: boolean } = {},
967967
redirectCount = 0
968968
): Promise<SecureFetchResponse> {
969+
options.signal?.throwIfAborted()
969970
const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS
970971
const requestedMaxResponseBytes = options.maxResponseBytes
971972
const maxResponseBytes =
@@ -1014,6 +1015,7 @@ export async function secureFetchWithPinnedIP(
10141015

10151016
validateUrlWithDNS(redirectUrl, 'redirectUrl', { allowHttp: options.allowHttp })
10161017
.then((validation) => {
1018+
options.signal?.throwIfAborted()
10171019
if (!validation.isValid) {
10181020
settledReject(new Error(`Redirect blocked: ${validation.error}`))
10191021
return

apps/sim/lib/core/security/secure-fetch-response-cap.server.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,18 @@ vi.mock('@/lib/core/config/env-flags', () => ({
1616
getProxyUrl: () => undefined,
1717
}))
1818

19+
import { resolveHostAddresses } from '@sim/security/dns'
1920
import {
2021
DEFAULT_MAX_RESPONSE_BYTES,
2122
secureFetchWithPinnedIP,
2223
} from '@/lib/core/security/input-validation.server'
2324

2425
const servers: http.Server[] = []
26+
const mockResolveHostAddresses = vi.mocked(resolveHostAddresses)
2527

2628
afterEach(() => {
2729
for (const server of servers.splice(0)) server.close()
30+
vi.clearAllMocks()
2831
})
2932

3033
/** Starts a throwaway loopback server and returns its origin. */
@@ -36,6 +39,49 @@ async function startServer(handler: http.RequestListener): Promise<string> {
3639
}
3740

3841
describe('secureFetchWithPinnedIP response cap', () => {
42+
it('does not open a request when the signal is already aborted', async () => {
43+
let requests = 0
44+
const origin = await startServer((_req, res) => {
45+
requests += 1
46+
res.end('unexpected')
47+
})
48+
const controller = new AbortController()
49+
controller.abort(new Error('cancelled'))
50+
51+
await expect(
52+
secureFetchWithPinnedIP(origin, '127.0.0.1', {
53+
allowHttp: true,
54+
signal: controller.signal,
55+
})
56+
).rejects.toThrow('cancelled')
57+
expect(requests).toBe(0)
58+
})
59+
60+
it('does not follow a redirect when cancellation arrives during redirect DNS validation', async () => {
61+
let targetRequests = 0
62+
const targetOrigin = await startServer((_req, res) => {
63+
targetRequests += 1
64+
res.end('unexpected')
65+
})
66+
const redirectOrigin = await startServer((_req, res) => {
67+
res.writeHead(302, { Location: targetOrigin.replace('127.0.0.1', 'localhost') })
68+
res.end()
69+
})
70+
const controller = new AbortController()
71+
mockResolveHostAddresses.mockImplementationOnce(async () => {
72+
controller.abort(new Error('cancelled during redirect DNS'))
73+
return { addresses: ['127.0.0.1'], preferred: '127.0.0.1' }
74+
})
75+
76+
await expect(
77+
secureFetchWithPinnedIP(redirectOrigin, '127.0.0.1', {
78+
allowHttp: true,
79+
signal: controller.signal,
80+
})
81+
).rejects.toThrow('cancelled during redirect DNS')
82+
expect(targetRequests).toBe(0)
83+
})
84+
3985
it('rejects a body that exceeds an explicit cap instead of buffering it', async () => {
4086
const origin = await startServer((_req, res) => {
4187
res.writeHead(200, { 'Content-Type': 'application/octet-stream' })

0 commit comments

Comments
 (0)