Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions apps/sim/tools/request-transport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const PROBE_FILE = {
mimeType: 'text/plain',
data: 'data:text/plain;base64,cHJvYmU=',
} as const
const DOT_SEGMENT_ERROR = 'Tool request URL cannot contain "." or ".." path segments'
const EXCEL_MIME_TYPE = 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'

function createSchemaProbeParams(
Expand Down Expand Up @@ -79,6 +80,12 @@ function isAbsoluteHttpUrl(url: string): boolean {
}
}

function hasDotDotPathSegment(url: string): boolean {
const pathStart = url.indexOf('/', url.indexOf('//') + 2)
if (pathStart === -1) return false
return url.slice(pathStart).split(/[?#]/)[0].split('/').includes('..')
}

function createRequestTool(
url: string | ((params: Record<string, unknown>) => string)
): ToolConfig {
Expand Down Expand Up @@ -129,6 +136,40 @@ describe('external request transport', () => {
).toBe('https://example.com')
})

it.each([
'https://api.example.com/v0/inboxes/inbox_1/drafts/..',
'https://api.example.com/v0/inboxes/inbox_1/drafts/../../../v0/inboxes/other',
'https://api.example.com/v0/inboxes/inbox_1/drafts/.',
'https://api.example.com/v0/inboxes/inbox_1/drafts/%2e%2E',
'https://api.example.com/v0/inboxes/inbox_1/drafts/.%2e?force=true',
'https://api.example.com/v0/inboxes/inbox_1/drafts/.\t.',
'https://api.example.com/v0/inboxes/inbox_1\\drafts\\..',
'https://api.example.com/v0/inboxes/inbox_1/drafts/..\u0001',
' https://api.example.com/v0/inboxes/inbox_1/drafts/..\u0000 ',
])('rejects a URL whose path resolves a dot segment: %s', (url) => {
expect(() =>
prepareToolRequest(
createRequestTool(() => url),
{}
)
).toThrow(DOT_SEGMENT_ERROR)
})

it.each([
'https://my-app.vercel.app/v1/domains/example.com',
'https://api.example.com/v1/files/..foo/foo../.env',
'https://api.example.com/v1/search?path=../x#..',
'https://api.example.com/',
'https://api.example.com/v1/files/..\u00a0',
])('allows dots that are not whole path segments: %s', (url) => {
expect(
prepareToolRequest(
createRequestTool(() => url),
{}
).url
).toBe(url)
})

it.each([
['http_request', requestTool, { url: '/api/auth/oauth/token', method: 'GET' }],
['webhook_request', webhookRequestTool, { url: '/api/auth/oauth/token', body: {} }],
Expand Down Expand Up @@ -188,12 +229,16 @@ describe('dynamic external request registry invariant', () => {
isAbsoluteHttpUrl(url),
`${toolId} resolved ${url} outside the external HTTP transport`
).toBe(true)
expect(() =>
const prepare = () =>
prepareToolRequest(
createRequestTool(() => url),
{}
)
).not.toThrow()
if (hasDotDotPathSegment(url)) {
expect(prepare, `${toolId} dispatched ${url}`).toThrow(DOT_SEGMENT_ERROR)
} else {
expect(prepare, `${toolId} rejected ${url}`).not.toThrow()
}
}

if (observations.length === 0) continue
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/tools/request-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
type ToolConfig,
type ToolDefinition,
} from '@/tools/types'
import { assertNoDotPathSegments } from '@/tools/url-path'

const MODEL_INPUT_PROJECTION_ERROR_MESSAGE = 'Model input could not be safely projected'
const PRIVATE_MODEL_INPUT_EXTERNAL_URL_ERROR_MESSAGE =
Expand Down Expand Up @@ -232,6 +233,7 @@ function assertExternalRequestUrl(url: string): void {
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
throw new Error(EXTERNAL_REQUEST_URL_ERROR_MESSAGE)
}
assertNoDotPathSegments(url)
}

/** Materializes one external HTTP request after enforcing the model-input boundary. */
Expand Down
26 changes: 26 additions & 0 deletions apps/sim/tools/url-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,29 @@ export function safeUrlPathSegment(value: string | number | bigint, paramName: s

return encodeSegment(trimmed, paramName)
}

/** Matches a scheme and authority, which end at the first `/`, `\`, `?`, or `#` in a special-scheme URL. */
const SCHEME_AND_AUTHORITY = /^[a-z][a-z\d+.-]*:[\\/]*[^\\/?#]*/i

/**
* Rejects a request URL whose path carries a dot segment the WHATWG parser
* would resolve away. Mirrors the parser: boundary C0 controls and spaces,
* then tabs and newlines, are stripped, `\` separates segments like `/`, and
* `%2e` counts as a dot.
*
* @throws If the path contains a `.` or `..` segment in any spelling.
*/
export function assertNoDotPathSegments(url: string): void {
const path = url
.replace(/^[\u0000-\u0020]+|[\u0000-\u0020]+$/g, '')
.replace(/[\t\n\r]/g, '')
Comment thread
waleedlatif1 marked this conversation as resolved.
.replace(SCHEME_AND_AUTHORITY, '')
.split(/[?#]/, 1)[0]
const hasDotSegment = path.split(/[\\/]/).some((segment) => {
const decoded = segment.replace(/%2e/gi, '.')
return decoded === '.' || decoded === '..'
})
if (hasDotSegment) {
throw new Error('Tool request URL cannot contain "." or ".." path segments')
}
}
Loading