Skip to content

Commit 7da1283

Browse files
committed
fix(tools): reject dynamic Sim-origin paths
1 parent c8a463e commit 7da1283

2 files changed

Lines changed: 160 additions & 26 deletions

File tree

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

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ describe('tool self-hop audit', () => {
689689
expect(audit.violations).toEqual([])
690690
})
691691

692-
it('does not treat a dynamic hostname suffix as Sim-origin normalization', () => {
692+
it('fails closed when a dynamic suffix follows the Sim origin', () => {
693693
const audit = auditToolSelfHops(`
694694
import { getBaseUrl } from '@/lib/core/utils/urls'
695695
const tool = {
@@ -704,7 +704,12 @@ describe('tool self-hop audit', () => {
704704
}
705705
`)
706706

707-
expect(audit.violations).toEqual([])
707+
expect(audit.violations).toEqual([
708+
expect.objectContaining({
709+
toolId: 'test_tool',
710+
reason: 'unresolved-request-policy',
711+
}),
712+
])
708713
})
709714

710715
it('allows an API-shaped path interpolated with an external provider origin', () => {
@@ -839,6 +844,38 @@ describe('tool self-hop audit', () => {
839844
])
840845
})
841846

847+
it('rejects a dynamic path resolved against the Sim origin', () => {
848+
const audit = auditToolSelfHops(`
849+
import { getBaseUrl } from '@/lib/core/utils/urls'
850+
const tool = {
851+
id: 'test_tool',
852+
request: { url: (params) => new URL(params.path, getBaseUrl()), method: 'GET' },
853+
}
854+
`)
855+
856+
expect(audit.violations).toEqual([
857+
expect.objectContaining({
858+
toolId: 'test_tool',
859+
reason: 'unresolved-request-policy',
860+
}),
861+
])
862+
})
863+
864+
it('allows an encoded path segment resolved against a static non-API Sim path', () => {
865+
const audit = auditToolSelfHops(`
866+
import { getBaseUrl } from '@/lib/core/utils/urls'
867+
const tool = {
868+
id: 'test_tool',
869+
request: {
870+
url: (params) => new URL('/assets/' + encodeURIComponent(params.id), getBaseUrl()),
871+
method: 'GET',
872+
},
873+
}
874+
`)
875+
876+
expect(audit.violations).toEqual([])
877+
})
878+
842879
it('allows an uninspectable path helper after an explicit external origin', () => {
843880
const audit = auditToolSelfHops(`
844881
const tool = {

scripts/check-tool-request-boundary.ts

Lines changed: 121 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,8 @@ function hasExplicitExternalUrlPrefix(
702702
function expressionContainsUnresolvedUrlHelper(
703703
expression: SyntaxNode,
704704
resolver: SelfHopResolver,
705-
seen = new Set<string>()
705+
seen = new Set<string>(),
706+
unresolvedIdentifierIsUnsafe = false
706707
): boolean {
707708
const current = unwrapExpression(expression)
708709
if (hasExplicitExternalUrlPrefix(current, resolver)) return false
@@ -711,26 +712,51 @@ function expressionContainsUnresolvedUrlHelper(
711712
const key = `${resolver.file}:unresolved-url:${current.name}`
712713
if (seen.has(key)) return false
713714
const binding = resolveScopedIdentifier(current.name, resolver)
714-
if (!binding) return false
715+
if (!binding) return unresolvedIdentifierIsUnsafe
715716
const nextSeen = new Set(seen)
716717
nextSeen.add(key)
717-
return expressionContainsUnresolvedUrlHelper(binding.expression, binding.resolver, nextSeen)
718+
return expressionContainsUnresolvedUrlHelper(
719+
binding.expression,
720+
binding.resolver,
721+
nextSeen,
722+
unresolvedIdentifierIsUnsafe
723+
)
718724
}
719725

720726
if (current.type === 'ConditionalExpression') {
721727
return (
722728
(isSyntaxNode(current.consequent) &&
723-
expressionContainsUnresolvedUrlHelper(current.consequent, resolver, new Set(seen))) ||
729+
expressionContainsUnresolvedUrlHelper(
730+
current.consequent,
731+
resolver,
732+
new Set(seen),
733+
unresolvedIdentifierIsUnsafe
734+
)) ||
724735
(isSyntaxNode(current.alternate) &&
725-
expressionContainsUnresolvedUrlHelper(current.alternate, resolver, new Set(seen)))
736+
expressionContainsUnresolvedUrlHelper(
737+
current.alternate,
738+
resolver,
739+
new Set(seen),
740+
unresolvedIdentifierIsUnsafe
741+
))
726742
)
727743
}
728744
if (current.type === 'LogicalExpression') {
729745
return (
730746
(isSyntaxNode(current.left) &&
731-
expressionContainsUnresolvedUrlHelper(current.left, resolver, new Set(seen))) ||
747+
expressionContainsUnresolvedUrlHelper(
748+
current.left,
749+
resolver,
750+
new Set(seen),
751+
unresolvedIdentifierIsUnsafe
752+
)) ||
732753
(isSyntaxNode(current.right) &&
733-
expressionContainsUnresolvedUrlHelper(current.right, resolver, new Set(seen)))
754+
expressionContainsUnresolvedUrlHelper(
755+
current.right,
756+
resolver,
757+
new Set(seen),
758+
unresolvedIdentifierIsUnsafe
759+
))
734760
)
735761
}
736762
if (
@@ -739,9 +765,20 @@ function expressionContainsUnresolvedUrlHelper(
739765
isSyntaxNode(current.left)
740766
) {
741767
if (isSimOriginExpression(current.left, resolver) && isSyntaxNode(current.right)) {
742-
return expressionContainsUnresolvedUrlHelper(current.right, resolver, new Set(seen))
768+
return expressionContainsUnresolvedUrlHelper(current.right, resolver, new Set(seen), true)
769+
}
770+
if (unresolvedIdentifierIsUnsafe && isSyntaxNode(current.right)) {
771+
return (
772+
expressionContainsUnresolvedUrlHelper(current.left, resolver, new Set(seen), true) ||
773+
expressionContainsUnresolvedUrlHelper(current.right, resolver, new Set(seen), true)
774+
)
743775
}
744-
return expressionContainsUnresolvedUrlHelper(current.left, resolver, new Set(seen))
776+
return expressionContainsUnresolvedUrlHelper(
777+
current.left,
778+
resolver,
779+
new Set(seen),
780+
unresolvedIdentifierIsUnsafe
781+
)
745782
}
746783
if (
747784
current.type === 'TemplateLiteral' &&
@@ -753,17 +790,29 @@ function expressionContainsUnresolvedUrlHelper(
753790
current.quasis.length === current.expressions.length + 1
754791
) {
755792
const leading = getTemplateQuasiValue(current.quasis[0])
756-
if (leading !== '') return false
793+
if (leading !== '') {
794+
return (
795+
unresolvedIdentifierIsUnsafe &&
796+
current.expressions.some((part) =>
797+
expressionContainsUnresolvedUrlHelper(part, resolver, new Set(seen), true)
798+
)
799+
)
800+
}
757801
const origin = current.expressions[0]
758802
if (isSimOriginExpression(origin, resolver)) {
759803
const following = getTemplateQuasiValue(current.quasis[1])
760804
return (
761805
following === '' &&
762806
current.expressions.length > 1 &&
763-
expressionContainsUnresolvedUrlHelper(current.expressions[1], resolver, new Set(seen))
807+
expressionContainsUnresolvedUrlHelper(current.expressions[1], resolver, new Set(seen), true)
764808
)
765809
}
766-
return expressionContainsUnresolvedUrlHelper(origin, resolver, new Set(seen))
810+
return expressionContainsUnresolvedUrlHelper(
811+
origin,
812+
resolver,
813+
new Set(seen),
814+
unresolvedIdentifierIsUnsafe
815+
)
767816
}
768817

769818
if (current.type === 'CallExpression' || current.type === 'OptionalCallExpression') {
@@ -777,11 +826,17 @@ function expressionContainsUnresolvedUrlHelper(
777826
return false
778827
}
779828
if (URL_VALUE_WRAPPER_CALLS.has(callee.name)) {
829+
if (unresolvedIdentifierIsUnsafe && callee.name === 'encodeURIComponent') return false
780830
const firstArgument = Array.isArray(current.arguments)
781831
? current.arguments.find(isSyntaxNode)
782832
: undefined
783833
return firstArgument
784-
? expressionContainsUnresolvedUrlHelper(firstArgument, resolver, new Set(seen))
834+
? expressionContainsUnresolvedUrlHelper(
835+
firstArgument,
836+
resolver,
837+
new Set(seen),
838+
unresolvedIdentifierIsUnsafe
839+
)
785840
: false
786841
}
787842
const key = `${resolver.file}:unresolved-url-call:${callee.name}`
@@ -801,12 +856,18 @@ function expressionContainsUnresolvedUrlHelper(
801856
binding.expression,
802857
binding.resolver,
803858
nextSeen,
804-
argumentsList
859+
argumentsList,
860+
unresolvedIdentifierIsUnsafe
805861
)
806862
}
807863
const access = getStaticMemberAccess(callee)
808864
if (!access) return true
809-
return expressionContainsUnresolvedUrlHelper(access.target, resolver, new Set(seen))
865+
return expressionContainsUnresolvedUrlHelper(
866+
access.target,
867+
resolver,
868+
new Set(seen),
869+
unresolvedIdentifierIsUnsafe
870+
)
810871
}
811872

812873
if (current.type === 'NewExpression') {
@@ -827,22 +888,47 @@ function expressionContainsUnresolvedUrlHelper(
827888
const base = current.arguments.length > 1 ? current.arguments[1] : undefined
828889
if (isSyntaxNode(base)) {
829890
return isSimOriginExpression(base, resolver)
830-
? Boolean(path && expressionContainsUnresolvedUrlHelper(path, resolver, new Set(seen)))
831-
: expressionContainsUnresolvedUrlHelper(base, resolver, new Set(seen))
891+
? Boolean(
892+
path && expressionContainsUnresolvedUrlHelper(path, resolver, new Set(seen), true)
893+
)
894+
: expressionContainsUnresolvedUrlHelper(
895+
base,
896+
resolver,
897+
new Set(seen),
898+
unresolvedIdentifierIsUnsafe
899+
)
832900
}
833-
return path ? expressionContainsUnresolvedUrlHelper(path, resolver, new Set(seen)) : false
901+
return path
902+
? expressionContainsUnresolvedUrlHelper(
903+
path,
904+
resolver,
905+
new Set(seen),
906+
unresolvedIdentifierIsUnsafe
907+
)
908+
: false
834909
}
835910
return true
836911
}
837912

838913
if (current.type === 'MemberExpression' || current.type === 'OptionalMemberExpression') {
839914
const access = getStaticMemberAccess(current)
840915
return access
841-
? expressionContainsUnresolvedUrlHelper(access.target, resolver, new Set(seen))
842-
: false
916+
? expressionContainsUnresolvedUrlHelper(
917+
access.target,
918+
resolver,
919+
new Set(seen),
920+
unresolvedIdentifierIsUnsafe
921+
)
922+
: unresolvedIdentifierIsUnsafe
843923
}
844924
if (FUNCTION_NODE_TYPES.has(current.type)) {
845-
return functionContainsUnresolvedUrlHelper(current, resolver, seen)
925+
return functionContainsUnresolvedUrlHelper(
926+
current,
927+
resolver,
928+
seen,
929+
[],
930+
unresolvedIdentifierIsUnsafe
931+
)
846932
}
847933
return false
848934
}
@@ -851,7 +937,8 @@ function functionContainsUnresolvedUrlHelper(
851937
fn: SyntaxNode,
852938
resolver: SelfHopResolver,
853939
seen: ReadonlySet<string>,
854-
argumentsList: readonly ScopedExpression[] = []
940+
argumentsList: readonly ScopedExpression[] = [],
941+
unresolvedIdentifierIsUnsafe = false
855942
): boolean {
856943
const current = unwrapExpression(fn)
857944
if (!FUNCTION_NODE_TYPES.has(current.type)) return true
@@ -875,7 +962,12 @@ function functionContainsUnresolvedUrlHelper(
875962
if (current.type === 'ArrowFunctionExpression' && isSyntaxNode(current.body)) {
876963
const body = unwrapExpression(current.body)
877964
if (body.type !== 'BlockStatement') {
878-
return expressionContainsUnresolvedUrlHelper(body, localResolver, new Set(seen))
965+
return expressionContainsUnresolvedUrlHelper(
966+
body,
967+
localResolver,
968+
new Set(seen),
969+
unresolvedIdentifierIsUnsafe
970+
)
879971
}
880972
}
881973
let unresolved = false
@@ -884,7 +976,12 @@ function functionContainsUnresolvedUrlHelper(
884976
if (
885977
node.type === 'ReturnStatement' &&
886978
isSyntaxNode(node.argument) &&
887-
expressionContainsUnresolvedUrlHelper(node.argument, localResolver, new Set(seen))
979+
expressionContainsUnresolvedUrlHelper(
980+
node.argument,
981+
localResolver,
982+
new Set(seen),
983+
unresolvedIdentifierIsUnsafe
984+
)
888985
) {
889986
unresolved = true
890987
return

0 commit comments

Comments
 (0)