Skip to content

Commit 14abca9

Browse files
committed
fix(tools): reject dot path segments in tool request URLs
1 parent 1f5001c commit 14abca9

3 files changed

Lines changed: 74 additions & 6 deletions

File tree

‎apps/sim/tools/request-transport.test.ts‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getErrorMessage } from '@sim/utils/errors'
12
import { describe, expect, it, vi } from 'vitest'
23
import { isInternalToolOperationRegistered } from '@/lib/internal/tool-operations/registry.server'
34
import { requestTool } from '@/tools/http/request'
@@ -32,6 +33,7 @@ const PROBE_FILE = {
3233
mimeType: 'text/plain',
3334
data: 'data:text/plain;base64,cHJvYmU=',
3435
} as const
36+
const DOT_SEGMENT_ERROR = 'Tool request URL cannot contain "." or ".." path segments'
3537
const EXCEL_MIME_TYPE = 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'
3638

3739
function createSchemaProbeParams(
@@ -129,6 +131,37 @@ describe('external request transport', () => {
129131
).toBe('https://example.com')
130132
})
131133

134+
it.each([
135+
'https://api.example.com/v0/inboxes/inbox_1/drafts/..',
136+
'https://api.example.com/v0/inboxes/inbox_1/drafts/../../../v0/inboxes/other',
137+
'https://api.example.com/v0/inboxes/inbox_1/drafts/.',
138+
'https://api.example.com/v0/inboxes/inbox_1/drafts/%2e%2E',
139+
'https://api.example.com/v0/inboxes/inbox_1/drafts/.%2e?force=true',
140+
'https://api.example.com/v0/inboxes/inbox_1/drafts/.\t.',
141+
'https://api.example.com/v0/inboxes/inbox_1\\drafts\\..',
142+
])('rejects a URL whose path resolves a dot segment: %s', (url) => {
143+
expect(() =>
144+
prepareToolRequest(
145+
createRequestTool(() => url),
146+
{}
147+
)
148+
).toThrow(DOT_SEGMENT_ERROR)
149+
})
150+
151+
it.each([
152+
'https://my-app.vercel.app/v1/domains/example.com',
153+
'https://api.example.com/v1/files/..foo/foo../.env',
154+
'https://api.example.com/v1/search?path=../x#..',
155+
'https://api.example.com/',
156+
])('allows dots that are not whole path segments: %s', (url) => {
157+
expect(
158+
prepareToolRequest(
159+
createRequestTool(() => url),
160+
{}
161+
).url
162+
).toBe(url)
163+
})
164+
132165
it.each([
133166
['http_request', requestTool, { url: '/api/auth/oauth/token', method: 'GET' }],
134167
['webhook_request', webhookRequestTool, { url: '/api/auth/oauth/token', body: {} }],
@@ -169,12 +202,12 @@ describe('dynamic external request registry invariant', () => {
169202
if (typeof urlBuilder !== 'function') throw new Error(`${toolId} must have a dynamic URL`)
170203
const observations: string[] = []
171204
const scenarios = [
172-
createSchemaProbeParams(tool, false),
173-
createSchemaProbeParams(tool, true),
174-
createSchemaProbeParams(tool, true, true),
205+
{ params: createSchemaProbeParams(tool, false), adversarial: false },
206+
{ params: createSchemaProbeParams(tool, true), adversarial: false },
207+
{ params: createSchemaProbeParams(tool, true, true), adversarial: true },
175208
]
176209

177-
for (const params of scenarios) {
210+
for (const { params, adversarial } of scenarios) {
178211
let url: string
179212
try {
180213
url = urlBuilder(params as never)
@@ -188,12 +221,20 @@ describe('dynamic external request registry invariant', () => {
188221
isAbsoluteHttpUrl(url),
189222
`${toolId} resolved ${url} outside the external HTTP transport`
190223
).toBe(true)
191-
expect(() =>
224+
const prepare = () =>
192225
prepareToolRequest(
193226
createRequestTool(() => url),
194227
{}
195228
)
196-
).not.toThrow()
229+
if (!adversarial) {
230+
expect(prepare, `${toolId} rejected ${url}`).not.toThrow()
231+
continue
232+
}
233+
try {
234+
prepare()
235+
} catch (error) {
236+
expect(getErrorMessage(error), `${toolId} rejected ${url}`).toBe(DOT_SEGMENT_ERROR)
237+
}
197238
}
198239

199240
if (observations.length === 0) continue

‎apps/sim/tools/request-transport.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type ToolConfig,
1111
type ToolDefinition,
1212
} from '@/tools/types'
13+
import { assertNoDotPathSegments } from '@/tools/url-path'
1314

1415
const MODEL_INPUT_PROJECTION_ERROR_MESSAGE = 'Model input could not be safely projected'
1516
const PRIVATE_MODEL_INPUT_EXTERNAL_URL_ERROR_MESSAGE =
@@ -232,6 +233,7 @@ function assertExternalRequestUrl(url: string): void {
232233
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
233234
throw new Error(EXTERNAL_REQUEST_URL_ERROR_MESSAGE)
234235
}
236+
assertNoDotPathSegments(url)
235237
}
236238

237239
/** Materializes one external HTTP request after enforcing the model-input boundary. */

‎apps/sim/tools/url-path.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,28 @@ export function safeUrlPathSegment(value: string | number | bigint, paramName: s
186186

187187
return encodeSegment(trimmed, paramName)
188188
}
189+
190+
/** Matches a scheme and authority, which end at the first `/`, `\`, `?`, or `#` in a special-scheme URL. */
191+
const SCHEME_AND_AUTHORITY = /^[a-z][a-z\d+.-]*:[\\/]*[^\\/?#]*/i
192+
193+
/**
194+
* Rejects a request URL whose path carries a dot segment the WHATWG parser
195+
* would resolve away. Mirrors the parser: tabs and newlines are stripped,
196+
* `\` separates segments like `/`, and `%2e` counts as a dot.
197+
*
198+
* @throws If the path contains a `.` or `..` segment in any spelling.
199+
*/
200+
export function assertNoDotPathSegments(url: string): void {
201+
const path = url
202+
.replace(/[\t\n\r]/g, '')
203+
.trim()
204+
.replace(SCHEME_AND_AUTHORITY, '')
205+
.split(/[?#]/, 1)[0]
206+
const hasDotSegment = path.split(/[\\/]/).some((segment) => {
207+
const decoded = segment.replace(/%2e/gi, '.')
208+
return decoded === '.' || decoded === '..'
209+
})
210+
if (hasDotSegment) {
211+
throw new Error('Tool request URL cannot contain "." or ".." path segments')
212+
}
213+
}

0 commit comments

Comments
 (0)