Skip to content

Commit 0cb9a8d

Browse files
committed
fix(box_sign,twilio): guard the ids the sweep's scoping missed
box_sign is a separate tools directory exposed by the Box block, so a sweep scoped by block name skipped it; signRequestId had no guard and no encoder, and a '.' turned get-request into list-every-sign-request. twilio get-recording built its provider URL inside an internal route, so probing request.url could not see it. The existing accountSid.startsWith('AC') check passes 'AC.../../..'; both ids now must match a real 34-char SID. Also caps the media download at 7MB, below the 10MB response ceiling base64 inflation would otherwise blow past.
1 parent 1d60a2e commit 0cb9a8d

6 files changed

Lines changed: 406 additions & 6 deletions

File tree

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Guards the provider URL this route builds from caller-supplied values.
5+
*
6+
* `accountSid` and `recordingSid` are NOT credential-derived — the tool body is
7+
* `{accountSid, authToken, recordingSid}` (`tools/twilio_voice/get_recording.ts`)
8+
* and `recordingSid` is `visibility: 'user-or-llm'`, so prompt injection controls
9+
* it. The contract only enforces `.min(1)`, and `validateUrlWithDNS` pins the
10+
* *host*, not the path. Before the guard, `recordingSid = '../Messages'` resolved
11+
* `/2010-04-01/Accounts/{acct}/Recordings/../Messages.json` down to
12+
* `/2010-04-01/Accounts/{acct}/Messages.json`; the route then follows `data.uri`,
13+
* refetches it with the caller's Basic auth, and returns the body base64-encoded
14+
* as a `file` output — an arbitrary authenticated GET across the caller's Twilio
15+
* account, exfiltrated as an attachment.
16+
*
17+
* Every URL assertion resolves through `new URL(...)` — the same normalization
18+
* `fetch` performs — and compares the full decoded `pathname` segment list.
19+
*/
20+
import {
21+
createMockRequest,
22+
hybridAuthMockFns,
23+
inputValidationMock,
24+
inputValidationMockFns,
25+
} from '@sim/testing'
26+
import { beforeEach, describe, expect, it, vi } from 'vitest'
27+
28+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
29+
30+
import { MAX_TWILIO_RECORDING_BYTES, POST } from '@/app/api/tools/twilio/get-recording/route'
31+
32+
const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
33+
34+
const PINNED_IP = '3.89.10.20'
35+
36+
/** A real Twilio SID: a two-letter prefix followed by 32 hexadecimal digits. */
37+
const ACCOUNT_SID = `AC${'a'.repeat(32)}`
38+
const RECORDING_SID = 'RE0123456789abcdef0123456789abcdef'
39+
40+
const baseBody = {
41+
accountSid: ACCOUNT_SID,
42+
authToken: 'auth-token',
43+
recordingSid: RECORDING_SID,
44+
}
45+
46+
function jsonResponse(body: unknown, ok = true) {
47+
return {
48+
ok,
49+
status: ok ? 200 : 400,
50+
statusText: '',
51+
headers: new Headers(),
52+
body: null,
53+
text: async () => JSON.stringify(body),
54+
json: async () => body,
55+
arrayBuffer: async () => new ArrayBuffer(0),
56+
}
57+
}
58+
59+
function mediaResponse(bytes: number) {
60+
return {
61+
ok: true,
62+
status: 200,
63+
statusText: '',
64+
headers: new Headers({ 'content-type': 'audio/wav' }),
65+
body: null,
66+
text: async () => '',
67+
json: async () => ({}),
68+
arrayBuffer: async () => new ArrayBuffer(bytes),
69+
}
70+
}
71+
72+
const recordingPayload = {
73+
sid: RECORDING_SID,
74+
call_sid: 'CA0123456789abcdef0123456789abcdef',
75+
duration: '12',
76+
status: 'completed',
77+
uri: `/2010-04-01/Accounts/${ACCOUNT_SID}/Recordings/${RECORDING_SID}.json`,
78+
}
79+
80+
beforeEach(() => {
81+
vi.clearAllMocks()
82+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
83+
success: true,
84+
userId: 'user-1',
85+
authType: 'internal_jwt',
86+
})
87+
mockValidateUrlWithDNS.mockResolvedValue({
88+
isValid: true,
89+
resolvedIP: PINNED_IP,
90+
originalHostname: 'api.twilio.com',
91+
})
92+
})
93+
94+
/** Vectors that must never reach `secureFetchWithPinnedIP` at all. */
95+
const REJECTED_SIDS = [
96+
'..',
97+
'.',
98+
' .. ',
99+
'%2e%2e',
100+
'a/b',
101+
'../Messages',
102+
'..\\..',
103+
'RE0123456789abcdef0123456789abcdef/../../Messages',
104+
'RE0123456789abcdef0123456789abcdef?PageSize=1000',
105+
'RE0123456789abcdef0123456789abcdef.json',
106+
'RE0123456789abcdef0123456789abcdefEXTRA',
107+
'REzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz',
108+
] as const
109+
110+
describe('POST /api/tools/twilio/get-recording — recordingSid guard', () => {
111+
it.each(REJECTED_SIDS)(
112+
'rejects recordingSid %j with a 400 and issues no request',
113+
async (sid) => {
114+
const response = await POST(createMockRequest('POST', { ...baseBody, recordingSid: sid }))
115+
116+
expect(response.status).toBe(400)
117+
const data = (await response.json()) as { success: boolean; error: string }
118+
expect(data.success).toBe(false)
119+
expect(data.error).toMatch(/Recording SID/i)
120+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
121+
}
122+
)
123+
124+
it.each(['..', 'a/b', `AC${'0123456789abcdef'.repeat(2)}/../..`, 'ACnothex'] as const)(
125+
'rejects accountSid %j with a 400 and issues no request',
126+
async (sid) => {
127+
const response = await POST(createMockRequest('POST', { ...baseBody, accountSid: sid }))
128+
129+
expect(response.status).toBe(400)
130+
const data = (await response.json()) as { success: boolean; error: string }
131+
expect(data.success).toBe(false)
132+
expect(data.error).toMatch(/Account SID/i)
133+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
134+
}
135+
)
136+
})
137+
138+
describe('POST /api/tools/twilio/get-recording — legitimate values', () => {
139+
it('builds byte-identical Twilio URLs for real SIDs', async () => {
140+
mockSecureFetchWithPinnedIP
141+
.mockResolvedValueOnce(jsonResponse(recordingPayload))
142+
.mockResolvedValueOnce(jsonResponse({ transcriptions: [] }))
143+
.mockResolvedValueOnce(mediaResponse(2048))
144+
145+
const response = await POST(createMockRequest('POST', baseBody))
146+
expect(response.status).toBe(200)
147+
148+
const infoUrl = mockSecureFetchWithPinnedIP.mock.calls[0][0] as string
149+
expect(infoUrl).toBe(
150+
`https://api.twilio.com/2010-04-01/Accounts/${ACCOUNT_SID}/Recordings/${RECORDING_SID}.json`
151+
)
152+
const resolvedInfo = new URL(infoUrl)
153+
expect(resolvedInfo.origin).toBe('https://api.twilio.com')
154+
expect(resolvedInfo.search).toBe('')
155+
expect(resolvedInfo.pathname.split('/').map(decodeURIComponent)).toEqual([
156+
'',
157+
'2010-04-01',
158+
'Accounts',
159+
ACCOUNT_SID,
160+
'Recordings',
161+
`${RECORDING_SID}.json`,
162+
])
163+
164+
const transcriptionUrl = new URL(mockSecureFetchWithPinnedIP.mock.calls[1][0] as string)
165+
expect(transcriptionUrl.pathname.split('/').map(decodeURIComponent)).toEqual([
166+
'',
167+
'2010-04-01',
168+
'Accounts',
169+
ACCOUNT_SID,
170+
'Transcriptions.json',
171+
])
172+
expect(transcriptionUrl.searchParams.get('RecordingSid')).toBe(RECORDING_SID)
173+
174+
const data = (await response.json()) as { success: boolean; output: { file: { size: number } } }
175+
expect(data.success).toBe(true)
176+
expect(data.output.file.size).toBe(2048)
177+
})
178+
179+
it('keeps a Twilio-supplied sid inside the query zone via URLSearchParams', async () => {
180+
mockSecureFetchWithPinnedIP
181+
.mockResolvedValueOnce(jsonResponse({ ...recordingPayload, sid: 'RE&PageSize=1000' }))
182+
.mockResolvedValueOnce(jsonResponse({ transcriptions: [] }))
183+
.mockResolvedValueOnce(mediaResponse(16))
184+
185+
await POST(createMockRequest('POST', baseBody))
186+
187+
const transcriptionUrl = new URL(mockSecureFetchWithPinnedIP.mock.calls[1][0] as string)
188+
expect(transcriptionUrl.searchParams.get('RecordingSid')).toBe('RE&PageSize=1000')
189+
expect(transcriptionUrl.searchParams.get('PageSize')).toBeNull()
190+
expect([...transcriptionUrl.searchParams.keys()]).toEqual(['RecordingSid'])
191+
})
192+
})
193+
194+
describe('POST /api/tools/twilio/get-recording — media size cap', () => {
195+
it('caps the media download at the size the JSON transport can actually carry', async () => {
196+
mockSecureFetchWithPinnedIP
197+
.mockResolvedValueOnce(jsonResponse(recordingPayload))
198+
.mockResolvedValueOnce(jsonResponse({ transcriptions: [] }))
199+
.mockResolvedValueOnce(mediaResponse(64))
200+
201+
await POST(createMockRequest('POST', baseBody))
202+
203+
expect(MAX_TWILIO_RECORDING_BYTES).toBeLessThanOrEqual(7.5 * 1024 * 1024)
204+
expect(mockSecureFetchWithPinnedIP.mock.calls[2][2]).toMatchObject({
205+
maxResponseBytes: MAX_TWILIO_RECORDING_BYTES,
206+
})
207+
})
208+
})

apps/sim/app/api/tools/twilio/get-recording/route.ts

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,40 @@ export const dynamic = 'force-dynamic'
1616

1717
const logger = createLogger('TwilioGetRecordingAPI')
1818

19+
/**
20+
* Shape of a Twilio resource identifier.
21+
*
22+
* Twilio documents every resource id as a 34-character String Identifier: a
23+
* two-letter prefix followed by 32 hexadecimal digits. No `/`, `\`, dot
24+
* segment, `?`, or `#` is legal in one, so pinning the shape has zero
25+
* false-rejection risk while closing the path zone entirely.
26+
*
27+
* This matters because both `accountSid` and `recordingSid` arrive in the
28+
* request body rather than from a credential, and `recordingSid` is
29+
* `visibility: 'user-or-llm'` on the calling tool. `validateUrlWithDNS` pins
30+
* the *host*, not the path, so an unguarded `recordingSid` of `../Messages`
31+
* re-aimed this route at an arbitrary resource in the caller's account — which
32+
* it then refetched with the caller's Basic auth and returned base64-encoded as
33+
* a `file` output.
34+
*/
35+
const ACCOUNT_SID_PATTERN = /^AC[0-9a-fA-F]{32}$/
36+
const RECORDING_SID_PATTERN = /^RE[0-9a-fA-F]{32}$/
37+
38+
/**
39+
* Ceiling on the downloaded recording media.
40+
*
41+
* This route returns the media base64-encoded inside its JSON body, and the
42+
* executor reads an internal tool response through `readToolResponseBody`,
43+
* which caps at `MAX_TOOL_RESPONSE_BODY_BYTES` (10 MB). Base64 inflates by 4/3,
44+
* so the largest recording that can survive the round trip is ~7.5 MB of raw
45+
* audio. Inheriting `DEFAULT_MAX_RESPONSE_BYTES` (100 MB) meant anything larger
46+
* downloaded and encoded in full — peaking at hundreds of MB of live
47+
* allocation — only for the executor to reject the body afterwards with "Tool
48+
* response size limit exceeded". Capping at the reachable size makes the
49+
* transport limit enforce itself while the bytes are still streaming.
50+
*/
51+
export const MAX_TWILIO_RECORDING_BYTES = 7 * 1024 * 1024
52+
1953
interface TwilioRecordingResponse {
2054
sid?: string
2155
call_sid?: string
@@ -67,11 +101,21 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
67101
if (!parsed.success) return parsed.response
68102
const { accountSid, authToken, recordingSid } = parsed.data.body
69103

70-
if (!accountSid.startsWith('AC')) {
104+
if (!ACCOUNT_SID_PATTERN.test(accountSid)) {
105+
return NextResponse.json(
106+
{
107+
success: false,
108+
error: `Invalid Account SID format. Account SID must be "AC" followed by 32 hexadecimal digits (you provided: ${accountSid.substring(0, 2)}...)`,
109+
},
110+
{ status: 400 }
111+
)
112+
}
113+
114+
if (!RECORDING_SID_PATTERN.test(recordingSid)) {
71115
return NextResponse.json(
72116
{
73117
success: false,
74-
error: `Invalid Account SID format. Account SID must start with "AC" (you provided: ${accountSid.substring(0, 2)}...)`,
118+
error: `Invalid Recording SID format. Recording SID must be "RE" followed by 32 hexadecimal digits (you provided: ${recordingSid.substring(0, 2)}...)`,
75119
},
76120
{ status: 400 }
77121
)
@@ -134,7 +178,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
134178
| undefined
135179

136180
try {
137-
const transcriptionUrl = `https://api.twilio.com/2010-04-01/Accounts/${accountSid}/Transcriptions.json?RecordingSid=${data.sid}`
181+
const transcriptionQuery = new URLSearchParams({ RecordingSid: data.sid ?? recordingSid })
182+
const transcriptionUrl = `https://api.twilio.com/2010-04-01/Accounts/${accountSid}/Transcriptions.json?${transcriptionQuery}`
138183
logger.info(`[${requestId}] Checking for transcriptions`)
139184

140185
const transcriptionUrlValidation = await validateUrlWithDNS(
@@ -182,6 +227,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
182227
{
183228
method: 'GET',
184229
headers: { Authorization: `Basic ${twilioAuth}` },
230+
maxResponseBytes: MAX_TWILIO_RECORDING_BYTES,
185231
}
186232
)
187233

apps/sim/tools/box_sign/cancel_request.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { ToolConfig } from '@/tools/types'
2+
import { safeUrlPathSegment } from '@/tools/url-path'
23
import type { BoxSignCancelRequestParams, BoxSignResponse } from './types'
34
import { SIGN_REQUEST_OUTPUT_PROPERTIES } from './types'
45

@@ -29,7 +30,8 @@ export const boxSignCancelRequestTool: ToolConfig<BoxSignCancelRequestParams, Bo
2930
},
3031

3132
request: {
32-
url: (params) => `https://api.box.com/2.0/sign_requests/${params.signRequestId}/cancel`,
33+
url: (params) =>
34+
`https://api.box.com/2.0/sign_requests/${safeUrlPathSegment(params.signRequestId, 'signRequestId')}/cancel`,
3335
method: 'POST',
3436
headers: (params) => ({
3537
Authorization: `Bearer ${params.accessToken}`,

apps/sim/tools/box_sign/get_request.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { ToolConfig } from '@/tools/types'
2+
import { safeUrlPathSegment } from '@/tools/url-path'
23
import type { BoxSignGetRequestParams, BoxSignResponse } from './types'
34
import { SIGN_REQUEST_OUTPUT_PROPERTIES } from './types'
45

@@ -29,7 +30,8 @@ export const boxSignGetRequestTool: ToolConfig<BoxSignGetRequestParams, BoxSignR
2930
},
3031

3132
request: {
32-
url: (params) => `https://api.box.com/2.0/sign_requests/${params.signRequestId}`,
33+
url: (params) =>
34+
`https://api.box.com/2.0/sign_requests/${safeUrlPathSegment(params.signRequestId, 'signRequestId')}`,
3335
method: 'GET',
3436
headers: (params) => ({
3537
Authorization: `Bearer ${params.accessToken}`,

0 commit comments

Comments
 (0)