Skip to content

Commit 0b9dbcc

Browse files
committed
fix(tools): normalize relative self hop paths
1 parent b9e4bdd commit 0b9dbcc

2 files changed

Lines changed: 71 additions & 14 deletions

File tree

scripts/check-tool-request-boundary.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,33 @@ describe('tool self-hop audit', () => {
409409
expect(audit.violations[0]?.reason).toBe('same-origin-tool-request')
410410
})
411411

412+
it('rejects a relative internal path resolved against the Sim origin', () => {
413+
const audit = auditToolSelfHops(`
414+
import { getBaseUrl } from '@/lib/core/utils/urls'
415+
const tool = {
416+
id: 'test_tool',
417+
request: { url: () => new URL('api/tools/test', getBaseUrl()).toString(), method: 'POST' },
418+
}
419+
`)
420+
421+
expect(audit.violations[0]?.reason).toBe('same-origin-tool-request')
422+
})
423+
424+
it('rejects a normalized relative internal path resolved against the Sim origin', () => {
425+
const audit = auditToolSelfHops(`
426+
import { getBaseUrl } from '@/lib/core/utils/urls'
427+
function buildPath() {
428+
return 'provider/../api/tools/test'
429+
}
430+
const tool = {
431+
id: 'test_tool',
432+
request: { url: () => new URL(buildPath(), getBaseUrl()).toString(), method: 'POST' },
433+
}
434+
`)
435+
436+
expect(audit.violations[0]?.reason).toBe('same-origin-tool-request')
437+
})
438+
412439
it('rejects an internal path resolved against a path-normalized Sim origin', () => {
413440
const audit = auditToolSelfHops(`
414441
import { getBaseUrl } from '@/lib/core/utils/urls'
@@ -623,6 +650,21 @@ describe('tool self-hop audit', () => {
623650
expect(audit.violations).toEqual([])
624651
})
625652

653+
it('allows a protocol-relative provider URL resolved against the Sim origin', () => {
654+
const audit = auditToolSelfHops(`
655+
import { getBaseUrl } from '@/lib/core/utils/urls'
656+
const tool = {
657+
id: 'test_tool',
658+
request: {
659+
url: () => new URL('//provider.example.com/api/messages', getBaseUrl()).toString(),
660+
method: 'POST',
661+
},
662+
}
663+
`)
664+
665+
expect(audit.violations).toEqual([])
666+
})
667+
626668
it('does not treat hostname mutation as Sim-origin normalization', () => {
627669
const audit = auditToolSelfHops(`
628670
import { getBaseUrl } from '@/lib/core/utils/urls'

scripts/check-tool-request-boundary.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,14 @@ function collectFunctionLocalBindings(fn: SyntaxNode, locals: Map<string, Syntax
250250
function isInternalPathExpression(
251251
expression: SyntaxNode,
252252
resolver: SelfHopResolver,
253-
seen = new Set<string>()
253+
seen = new Set<string>(),
254+
allowRelative = false
254255
): boolean {
255256
const current = unwrapExpression(expression)
256257
const staticValue = getStaticString(current)
257-
if (staticValue?.startsWith('/api/')) return true
258+
if (staticValue && isInternalApiPath(staticValue, allowRelative)) return true
258259
const prefix = getStringPrefix(current)
259-
if (prefix?.startsWith('/api/')) return true
260+
if (prefix && isInternalApiPath(prefix, allowRelative)) return true
260261

261262
if (current.type === 'Identifier' && typeof current.name === 'string') {
262263
const key = `${resolver.file}:path:${current.name}`
@@ -265,7 +266,7 @@ function isInternalPathExpression(
265266
if (!binding) return false
266267
const nextSeen = new Set(seen)
267268
nextSeen.add(key)
268-
return isInternalPathExpression(binding.expression, binding.resolver, nextSeen)
269+
return isInternalPathExpression(binding.expression, binding.resolver, nextSeen, allowRelative)
269270
}
270271

271272
if (current.type === 'CallExpression' || current.type === 'OptionalCallExpression') {
@@ -287,7 +288,8 @@ function isInternalPathExpression(
287288
binding.expression,
288289
binding.resolver,
289290
argumentsList,
290-
nextSeen
291+
nextSeen,
292+
allowRelative
291293
)
292294
}
293295
}
@@ -296,22 +298,22 @@ function isInternalPathExpression(
296298
if (current.type === 'ConditionalExpression') {
297299
return (
298300
(isSyntaxNode(current.consequent) &&
299-
isInternalPathExpression(current.consequent, resolver, new Set(seen))) ||
301+
isInternalPathExpression(current.consequent, resolver, new Set(seen), allowRelative)) ||
300302
(isSyntaxNode(current.alternate) &&
301-
isInternalPathExpression(current.alternate, resolver, new Set(seen)))
303+
isInternalPathExpression(current.alternate, resolver, new Set(seen), allowRelative))
302304
)
303305
}
304306
if (current.type === 'LogicalExpression') {
305307
return (
306308
(isSyntaxNode(current.left) &&
307-
isInternalPathExpression(current.left, resolver, new Set(seen))) ||
309+
isInternalPathExpression(current.left, resolver, new Set(seen), allowRelative)) ||
308310
(isSyntaxNode(current.right) &&
309-
isInternalPathExpression(current.right, resolver, new Set(seen)))
311+
isInternalPathExpression(current.right, resolver, new Set(seen), allowRelative))
310312
)
311313
}
312314
if (current.type === 'BinaryExpression' && current.operator === '+') {
313315
return isSyntaxNode(current.left)
314-
? isInternalPathExpression(current.left, resolver, new Set(seen))
316+
? isInternalPathExpression(current.left, resolver, new Set(seen), allowRelative)
315317
: false
316318
}
317319
return false
@@ -321,7 +323,8 @@ function functionReturnsInternalPath(
321323
fn: SyntaxNode,
322324
resolver: SelfHopResolver,
323325
argumentsList: readonly ScopedExpression[],
324-
seen: ReadonlySet<string>
326+
seen: ReadonlySet<string>,
327+
allowRelative: boolean
325328
): boolean {
326329
const current = unwrapExpression(fn)
327330
if (!FUNCTION_NODE_TYPES.has(current.type)) return false
@@ -345,7 +348,7 @@ function functionReturnsInternalPath(
345348
if (current.type === 'ArrowFunctionExpression' && isSyntaxNode(current.body)) {
346349
const body = unwrapExpression(current.body)
347350
if (body.type !== 'BlockStatement') {
348-
return isInternalPathExpression(body, localResolver, new Set(seen))
351+
return isInternalPathExpression(body, localResolver, new Set(seen), allowRelative)
349352
}
350353
}
351354
let found = false
@@ -354,7 +357,7 @@ function functionReturnsInternalPath(
354357
if (
355358
node.type === 'ReturnStatement' &&
356359
isSyntaxNode(node.argument) &&
357-
isInternalPathExpression(node.argument, localResolver, new Set(seen))
360+
isInternalPathExpression(node.argument, localResolver, new Set(seen), allowRelative)
358361
) {
359362
found = true
360363
return
@@ -365,6 +368,18 @@ function functionReturnsInternalPath(
365368
return found
366369
}
367370

371+
function isInternalApiPath(value: string, allowRelative: boolean): boolean {
372+
if (value.startsWith('/api/')) return true
373+
if (!allowRelative) return false
374+
try {
375+
const base = new URL('https://sim-boundary.invalid/')
376+
const resolved = new URL(value, base)
377+
return resolved.origin === base.origin && resolved.pathname.startsWith('/api/')
378+
} catch {
379+
return false
380+
}
381+
}
382+
368383
function isOriginPreservingStaticSuffix(expression: SyntaxNode): boolean {
369384
const suffix = getStaticString(expression)
370385
return suffix !== undefined && (suffix === '' || /^[/?#]/.test(suffix))
@@ -616,7 +631,7 @@ function isInternalUrlConstruction(node: SyntaxNode, resolver: SelfHopResolver):
616631
}
617632
return (
618633
isSyntaxNode(current.arguments[1]) &&
619-
isInternalPathExpression(current.arguments[0], resolver) &&
634+
isInternalPathExpression(current.arguments[0], resolver, new Set(), true) &&
620635
isSimOriginExpression(current.arguments[1], resolver)
621636
)
622637
}

0 commit comments

Comments
 (0)