Skip to content

Commit 0b47551

Browse files
committed
fix(tools): match URL parser boundary stripping in dot-segment check
1 parent 14abca9 commit 0b47551

2 files changed

Lines changed: 20 additions & 15 deletions

File tree

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { getErrorMessage } from '@sim/utils/errors'
21
import { describe, expect, it, vi } from 'vitest'
32
import { isInternalToolOperationRegistered } from '@/lib/internal/tool-operations/registry.server'
43
import { requestTool } from '@/tools/http/request'
@@ -81,6 +80,12 @@ function isAbsoluteHttpUrl(url: string): boolean {
8180
}
8281
}
8382

83+
function hasDotDotPathSegment(url: string): boolean {
84+
const pathStart = url.indexOf('/', url.indexOf('//') + 2)
85+
if (pathStart === -1) return false
86+
return url.slice(pathStart).split(/[?#]/)[0].split('/').includes('..')
87+
}
88+
8489
function createRequestTool(
8590
url: string | ((params: Record<string, unknown>) => string)
8691
): ToolConfig {
@@ -139,6 +144,8 @@ describe('external request transport', () => {
139144
'https://api.example.com/v0/inboxes/inbox_1/drafts/.%2e?force=true',
140145
'https://api.example.com/v0/inboxes/inbox_1/drafts/.\t.',
141146
'https://api.example.com/v0/inboxes/inbox_1\\drafts\\..',
147+
'https://api.example.com/v0/inboxes/inbox_1/drafts/..\u0001',
148+
' https://api.example.com/v0/inboxes/inbox_1/drafts/..\u0000 ',
142149
])('rejects a URL whose path resolves a dot segment: %s', (url) => {
143150
expect(() =>
144151
prepareToolRequest(
@@ -153,6 +160,7 @@ describe('external request transport', () => {
153160
'https://api.example.com/v1/files/..foo/foo../.env',
154161
'https://api.example.com/v1/search?path=../x#..',
155162
'https://api.example.com/',
163+
'https://api.example.com/v1/files/..\u00a0',
156164
])('allows dots that are not whole path segments: %s', (url) => {
157165
expect(
158166
prepareToolRequest(
@@ -202,12 +210,12 @@ describe('dynamic external request registry invariant', () => {
202210
if (typeof urlBuilder !== 'function') throw new Error(`${toolId} must have a dynamic URL`)
203211
const observations: string[] = []
204212
const scenarios = [
205-
{ params: createSchemaProbeParams(tool, false), adversarial: false },
206-
{ params: createSchemaProbeParams(tool, true), adversarial: false },
207-
{ params: createSchemaProbeParams(tool, true, true), adversarial: true },
213+
createSchemaProbeParams(tool, false),
214+
createSchemaProbeParams(tool, true),
215+
createSchemaProbeParams(tool, true, true),
208216
]
209217

210-
for (const { params, adversarial } of scenarios) {
218+
for (const params of scenarios) {
211219
let url: string
212220
try {
213221
url = urlBuilder(params as never)
@@ -226,14 +234,10 @@ describe('dynamic external request registry invariant', () => {
226234
createRequestTool(() => url),
227235
{}
228236
)
229-
if (!adversarial) {
237+
if (hasDotDotPathSegment(url)) {
238+
expect(prepare, `${toolId} dispatched ${url}`).toThrow(DOT_SEGMENT_ERROR)
239+
} else {
230240
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)
237241
}
238242
}
239243

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,16 @@ const SCHEME_AND_AUTHORITY = /^[a-z][a-z\d+.-]*:[\\/]*[^\\/?#]*/i
192192

193193
/**
194194
* 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.
195+
* would resolve away. Mirrors the parser: boundary C0 controls and spaces,
196+
* then tabs and newlines, are stripped, `\` separates segments like `/`, and
197+
* `%2e` counts as a dot.
197198
*
198199
* @throws If the path contains a `.` or `..` segment in any spelling.
199200
*/
200201
export function assertNoDotPathSegments(url: string): void {
201202
const path = url
203+
.replace(/^[\u0000-\u0020]+|[\u0000-\u0020]+$/g, '')
202204
.replace(/[\t\n\r]/g, '')
203-
.trim()
204205
.replace(SCHEME_AND_AUTHORITY, '')
205206
.split(/[?#]/, 1)[0]
206207
const hasDotSegment = path.split(/[\\/]/).some((segment) => {

0 commit comments

Comments
 (0)