Skip to content

Commit dd8d24f

Browse files
committed
improvement(brex): validate pre-signed upload URL with DNS pinning and harden API key input
1 parent 27ced43 commit dd8d24f

3 files changed

Lines changed: 91 additions & 33 deletions

File tree

apps/sim/app/api/tools/brex/upload-receipt/route.test.ts

Lines changed: 63 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { createMockRequest, hybridAuthMockFns } from '@sim/testing'
4+
import {
5+
createMockRequest,
6+
hybridAuthMockFns,
7+
inputValidationMock,
8+
inputValidationMockFns,
9+
} from '@sim/testing'
510
import { beforeEach, describe, expect, it, vi } from 'vitest'
611

712
const { mockProcessFilesToUserFiles, mockDownloadFileFromStorage, mockAssertToolFileAccess } =
@@ -11,6 +16,7 @@ const { mockProcessFilesToUserFiles, mockDownloadFileFromStorage, mockAssertTool
1116
mockAssertToolFileAccess: vi.fn(),
1217
}))
1318

19+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
1420
vi.mock('@/lib/uploads/utils/file-utils', () => ({
1521
processFilesToUserFiles: mockProcessFilesToUserFiles,
1622
}))
@@ -25,6 +31,8 @@ import { POST } from '@/app/api/tools/brex/upload-receipt/route'
2531

2632
const mockFetch = vi.fn()
2733

34+
const PINNED_IP = '52.216.0.1'
35+
2836
const baseBody = {
2937
apiKey: 'bxt_test_token',
3038
expenseId: 'expense_123',
@@ -48,6 +56,11 @@ beforeEach(() => {
4856
userId: 'user-1',
4957
authType: 'internal_jwt',
5058
})
59+
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
60+
isValid: true,
61+
resolvedIP: PINNED_IP,
62+
})
63+
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(jsonResponse({}))
5164
mockProcessFilesToUserFiles.mockReturnValue([
5265
{ key: 'uploads/receipt.pdf', name: 'receipt.pdf', size: 5, type: 'application/pdf' },
5366
])
@@ -68,11 +81,9 @@ describe('POST /api/tools/brex/upload-receipt', () => {
6881
})
6982

7083
it('creates a receipt upload for an expense and PUTs the file to the pre-signed URL', async () => {
71-
mockFetch
72-
.mockResolvedValueOnce(
73-
jsonResponse({ id: 'receipt_1', uri: 'https://s3.example.com/presigned' })
74-
)
75-
.mockResolvedValueOnce(jsonResponse({}))
84+
mockFetch.mockResolvedValueOnce(
85+
jsonResponse({ id: 'receipt_1', uri: 'https://s3.example.com/presigned' })
86+
)
7687

7788
const response = await POST(createMockRequest('POST', baseBody))
7889
expect(response.status).toBe(200)
@@ -82,15 +93,21 @@ describe('POST /api/tools/brex/upload-receipt', () => {
8293
output: { receiptId: 'receipt_1', receiptName: 'receipt.pdf', expenseId: 'expense_123' },
8394
})
8495

85-
expect(mockFetch).toHaveBeenCalledTimes(2)
96+
expect(mockFetch).toHaveBeenCalledTimes(1)
8697
const [createUrl, createInit] = mockFetch.mock.calls[0]
8798
expect(createUrl).toBe('https://api.brex.com/v1/expenses/card/expense_123/receipt_upload')
8899
expect(createInit.method).toBe('POST')
89100
expect(createInit.headers.Authorization).toBe('Bearer bxt_test_token')
90101
expect(JSON.parse(createInit.body)).toEqual({ receipt_name: 'receipt.pdf' })
91102

92-
const [uploadUrl, uploadInit] = mockFetch.mock.calls[1]
103+
expect(inputValidationMockFns.mockValidateUrlWithDNS).toHaveBeenCalledWith(
104+
'https://s3.example.com/presigned',
105+
'uri'
106+
)
107+
const [uploadUrl, pinnedIP, uploadInit] =
108+
inputValidationMockFns.mockSecureFetchWithPinnedIP.mock.calls[0]
93109
expect(uploadUrl).toBe('https://s3.example.com/presigned')
110+
expect(pinnedIP).toBe(PINNED_IP)
94111
expect(uploadInit.method).toBe('PUT')
95112
})
96113

@@ -101,11 +118,9 @@ describe('POST /api/tools/brex/upload-receipt', () => {
101118
})
102119

103120
it('trims a padded expense ID before building the upload URL', async () => {
104-
mockFetch
105-
.mockResolvedValueOnce(
106-
jsonResponse({ id: 'receipt_5', uri: 'https://s3.example.com/presigned' })
107-
)
108-
.mockResolvedValueOnce(jsonResponse({}))
121+
mockFetch.mockResolvedValueOnce(
122+
jsonResponse({ id: 'receipt_5', uri: 'https://s3.example.com/presigned' })
123+
)
109124

110125
const response = await POST(
111126
createMockRequest('POST', { ...baseBody, expenseId: ' expense_123 ' })
@@ -123,12 +138,18 @@ describe('POST /api/tools/brex/upload-receipt', () => {
123138
expect(mockFetch).not.toHaveBeenCalled()
124139
})
125140

141+
it('rejects an API key containing header-breaking characters', async () => {
142+
const response = await POST(
143+
createMockRequest('POST', { ...baseBody, apiKey: 'bxt_test\r\nX-Injected: 1' })
144+
)
145+
expect(response.status).toBe(400)
146+
expect(mockFetch).not.toHaveBeenCalled()
147+
})
148+
126149
it('uses receipt match when no expense ID is provided', async () => {
127-
mockFetch
128-
.mockResolvedValueOnce(
129-
jsonResponse({ id: 'receipt_2', uri: 'https://s3.example.com/presigned' })
130-
)
131-
.mockResolvedValueOnce(jsonResponse({}))
150+
mockFetch.mockResolvedValueOnce(
151+
jsonResponse({ id: 'receipt_2', uri: 'https://s3.example.com/presigned' })
152+
)
132153

133154
const response = await POST(
134155
createMockRequest('POST', { apiKey: 'bxt_test_token', file: baseBody.file })
@@ -146,11 +167,9 @@ describe('POST /api/tools/brex/upload-receipt', () => {
146167
})
147168

148169
it('honors a receipt name override', async () => {
149-
mockFetch
150-
.mockResolvedValueOnce(
151-
jsonResponse({ id: 'receipt_3', uri: 'https://s3.example.com/presigned' })
152-
)
153-
.mockResolvedValueOnce(jsonResponse({}))
170+
mockFetch.mockResolvedValueOnce(
171+
jsonResponse({ id: 'receipt_3', uri: 'https://s3.example.com/presigned' })
172+
)
154173

155174
const response = await POST(
156175
createMockRequest('POST', { ...baseBody, receiptName: 'march-dinner.pdf' })
@@ -169,6 +188,7 @@ describe('POST /api/tools/brex/upload-receipt', () => {
169188
expect(data.success).toBe(false)
170189
expect(data.error).toContain('Expense not found')
171190
expect(mockFetch).toHaveBeenCalledTimes(1)
191+
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
172192
})
173193

174194
it('rejects files over the 50 MB limit', async () => {
@@ -181,12 +201,27 @@ describe('POST /api/tools/brex/upload-receipt', () => {
181201
expect(mockFetch).not.toHaveBeenCalled()
182202
})
183203

204+
it('blocks pre-signed URLs that fail SSRF validation', async () => {
205+
mockFetch.mockResolvedValueOnce(
206+
jsonResponse({ id: 'receipt_6', uri: 'https://169.254.169.254/latest/meta-data' })
207+
)
208+
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValueOnce({
209+
isValid: false,
210+
error: 'uri resolves to a blocked IP address',
211+
})
212+
213+
const response = await POST(createMockRequest('POST', baseBody))
214+
expect(response.status).toBe(502)
215+
const data = await response.json()
216+
expect(data.error).toContain('invalid upload URL')
217+
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
218+
})
219+
184220
it('fails when the pre-signed upload fails', async () => {
185-
mockFetch
186-
.mockResolvedValueOnce(
187-
jsonResponse({ id: 'receipt_4', uri: 'https://s3.example.com/presigned' })
188-
)
189-
.mockResolvedValueOnce(jsonResponse({}, 403))
221+
mockFetch.mockResolvedValueOnce(
222+
jsonResponse({ id: 'receipt_4', uri: 'https://s3.example.com/presigned' })
223+
)
224+
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValueOnce(jsonResponse({}, 403))
190225

191226
const response = await POST(createMockRequest('POST', baseBody))
192227
expect(response.status).toBe(502)

apps/sim/app/api/tools/brex/upload-receipt/route.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { type NextRequest, NextResponse } from 'next/server'
44
import { brexUploadReceiptContract } from '@/lib/api/contracts/tools/brex'
55
import { parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
7+
import {
8+
secureFetchWithPinnedIP,
9+
validateUrlWithDNS,
10+
} from '@/lib/core/security/input-validation.server'
711
import { generateRequestId } from '@/lib/core/utils/request'
812
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
913
import { processFilesToUserFiles, type RawFileInput } from '@/lib/uploads/utils/file-utils'
@@ -93,10 +97,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
9397
)
9498
}
9599

96-
const uploadResponse = await fetch(createData.uri, {
97-
method: 'PUT',
98-
body: new Uint8Array(fileBuffer),
99-
})
100+
const uriValidation = await validateUrlWithDNS(createData.uri, 'uri')
101+
if (!uriValidation.isValid) {
102+
logger.error(`[${requestId}] Pre-signed upload URL failed SSRF validation:`, {
103+
error: uriValidation.error,
104+
})
105+
return NextResponse.json(
106+
{ success: false, error: 'Brex returned an invalid upload URL' },
107+
{ status: 502 }
108+
)
109+
}
110+
111+
const uploadResponse = await secureFetchWithPinnedIP(
112+
createData.uri,
113+
uriValidation.resolvedIP!,
114+
{
115+
method: 'PUT',
116+
body: new Uint8Array(fileBuffer),
117+
}
118+
)
100119

101120
if (!uploadResponse.ok) {
102121
logger.error(`[${requestId}] Receipt upload to pre-signed URL failed:`, {

apps/sim/lib/api/contracts/tools/brex.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { defineRouteContract } from '@/lib/api/contracts/types'
44
import { RawFileInputSchema } from '@/lib/uploads/utils/file-schemas'
55

66
export const brexUploadReceiptBodySchema = z.object({
7-
apiKey: z.string().min(1, 'API key is required').max(512, 'API key is too long'),
7+
apiKey: z
8+
.string()
9+
.min(1, 'API key is required')
10+
.max(512, 'API key is too long')
11+
.regex(/^[\x21-\x7e]+$/, 'API key contains invalid characters'),
812
expenseId: z
913
.string()
1014
.trim()

0 commit comments

Comments
 (0)