fix(tools): close remaining self-hop bypasses

Author: icecrasher321

apps/sim/lib/core/security/input-validation.server.ts

@@ -372,6 +372,8 @@ export interface SecureFetchOptions {
   stripAuthOnRedirect?: boolean
   /** Omit for the historical behavior used by existing workflows. */
   redirectPolicy?: HttpRedirectPolicy
+  /** Rejects a redirect target before DNS resolution or a follow-up request is attempted. */
+  assertRedirectTarget?: (url: string) => void
   /**
    * Pre-validated, IP-pinned `http://` proxy URL (see {@link validateAndPinProxyUrl}).
    * When set, the connection routes through this proxy and target-IP pinning is
@@ -1059,6 +1061,12 @@ export async function secureFetchWithPinnedIP(
         res.resume()
         const redirectUrl = resolveRedirectUrl(url, location)
 
+        try {
+          options.assertRedirectTarget?.(redirectUrl)
+        } catch (error) {
+          settledReject(error)
+          return
+        }
         validateUrlWithDNS(redirectUrl, 'redirectUrl', { allowHttp: options.allowHttp })
           .then((validation) => {
             if (!validation.isValid) {

apps/sim/lib/core/security/pinned-redirect-replay.server.test.ts

@@ -56,6 +56,29 @@ async function startRecordingServer(hops: RecordedHop[]): Promise<string> {
 }
 
 describe('secureFetchWithPinnedIP redirect replay', () => {
+  it('rejects a redirect target before following it', async () => {
+    const hops: RecordedHop[] = []
+    const target = await startRecordingServer(hops)
+    const origin = await startServer((req, res) => {
+      req.resume()
+      res.writeHead(302, { location: `${target}/after` })
+      res.end()
+    })
+    const assertRedirectTarget = vi.fn((url: string) => {
+      if (url === `${target}/after`) throw new Error('redirect target rejected')
+    })
+
+    await expect(
+      secureFetchWithPinnedIP(origin, '127.0.0.1', {
+        allowHttp: true,
+        assertRedirectTarget,
+      })
+    ).rejects.toThrow('redirect target rejected')
+
+    expect(assertRedirectTarget).toHaveBeenCalledWith(`${target}/after`)
+    expect(hops).toEqual([])
+  })
+
   it('preserves historical replay when no redirect policy is present', async () => {
     const hops: RecordedHop[] = []
     const target = await startRecordingServer(hops)

apps/sim/tools/index.test.ts

@@ -2674,6 +2674,11 @@ describe('Internal Route Trust', () => {
       'http://localhost:3000/api/v1/workflows/test',
       'toolUrl'
     )
+    expect(mockSecureFetchWithPinnedIP).toHaveBeenCalledWith(
+      'http://localhost:3000/api/v1/workflows/test',
+      '93.184.216.34',
+      expect.objectContaining({ assertRedirectTarget: undefined })
+    )
   })
 
   it('rejects an integration request that resolves back to this Sim instance', async () => {
@@ -2705,6 +2710,40 @@ describe('Internal Route Trust', () => {
     }
   })
 
+  it('rejects an integration redirect that resolves back to this Sim instance', async () => {
+    const mockTool = {
+      id: 'test_same_origin_redirect',
+      name: 'Same Origin Redirect Integration',
+      description: 'Regression fixture',
+      version: '1.0.0',
+      params: {},
+      request: {
+        url: () => 'https://api.example.com/download',
+        method: 'GET' as const,
+        headers: () => ({}),
+      },
+    }
+    ;(tools as Record<string, unknown>).test_same_origin_redirect = mockTool
+
+    try {
+      const result = await executeTool('test_same_origin_redirect', {})
+
+      expect(result.success).toBe(true)
+      const secureFetchOptions = mockSecureFetchWithPinnedIP.mock.calls.at(-1)?.[2]
+      expect(secureFetchOptions?.assertRedirectTarget).toBeTypeOf('function')
+      expect(() =>
+        secureFetchOptions?.assertRedirectTarget?.('http://localhost:3000/api/tools/test')
+      ).toThrow(
+        'External integration tools cannot target this Sim instance; use an internal operation'
+      )
+      expect(() =>
+        secureFetchOptions?.assertRedirectTarget?.('https://provider.example.com/download')
+      ).not.toThrow()
+    } finally {
+      Reflect.deleteProperty(tools, 'test_same_origin_redirect')
+    }
+  })
+
   it('transports only active provenance selected for an internal model input', async () => {
     const registry = new ResolvedSecretTraceRegistry([
       {

apps/sim/tools/index.ts

@@ -2686,6 +2686,14 @@ async function executeToolRequest(
           proxyUrl: proxyOption,
           stripAuthOnRedirect: requestParams.stripAuthOnRedirect,
           redirectPolicy: requestParams.redirectPolicy,
+          assertRedirectTarget:
+            tool.request.allowSameOrigin === true
+              ? undefined
+              : (redirectUrl) => {
+                  if (isSelfOriginUrl(redirectUrl)) {
+                    throw new Error(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE)
+                  }
+                },
         })
 
         const responseHeaders = new Headers(secureResponse.headers.toRecord())

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

@@ -632,6 +632,76 @@ describe('tool self-hop audit', () => {
     }
   )
 
+  it('resolves a constant tool ID before applying the same-origin allowlist', () => {
+    const audit = auditToolSelfHops(`
+      const TOOL_ID = 'http_request'
+      const tool = {
+        id: TOOL_ID,
+        request: {
+          allowSameOrigin: true,
+          url: (params) => params.url,
+          method: 'POST',
+          headers: () => ({}),
+        },
+      }
+    `)
+
+    expect(audit.violations).toEqual([])
+  })
+
+  it('audits a same-origin request when the tool ID is an expression', () => {
+    const audit = auditToolSelfHops(`
+      const tool = {
+        id: flag ? 'first_tool' : 'second_tool',
+        request: { url: '/api/tools/test', method: 'POST', headers: () => ({}) },
+      }
+    `)
+
+    expect(audit.violations).toEqual([
+      expect.objectContaining({ reason: 'same-origin-tool-request' }),
+    ])
+  })
+
+  it('fails closed on a computed request property key', () => {
+    const audit = auditToolSelfHops(`
+      const tool = {
+        id: 'test_tool',
+        [runtimeRequestKey]: {
+          url: 'https://provider.example.com',
+          method: 'POST',
+          headers: () => ({}),
+        },
+      }
+    `)
+
+    expect(audit.violations).toEqual([
+      expect.objectContaining({
+        toolId: 'test_tool',
+        reason: 'unresolved-request-policy',
+      }),
+    ])
+  })
+
+  it('fails closed on a computed request URL key', () => {
+    const audit = auditToolSelfHops(`
+      const tool = {
+        id: 'test_tool',
+        request: {
+          [runtimeUrlKey]: '/api/tools/test',
+          method: 'POST',
+          headers: () => ({}),
+        },
+      }
+    `)
+
+    expect(audit.violations).toEqual([
+      expect.objectContaining({
+        toolId: 'test_tool',
+        reason: 'unresolved-request-policy',
+      }),
+    ])
+  })
+
   it('rejects request.internal even when the URL comes only from a spread', () => {
     const audit = auditToolSelfHops(`
       const externalRequest = { url: 'https://api.example.com/v1/items' }

scripts/check-tool-request-boundary.ts

@@ -124,6 +124,7 @@ function unwrapExpression(expression: SyntaxNode): SyntaxNode {
 function getStaticPropertyName(property: SyntaxNode): string | undefined {
   if (!isSyntaxNode(property.key)) return undefined
   const key = property.key
+  if (property.computed === true) return getStaticString(key)
   if (key.type === 'Identifier' && typeof key.name === 'string') return key.name
   if (key.type === 'StringLiteral' && typeof key.value === 'string') return key.value
   return undefined
@@ -1278,11 +1279,28 @@ function functionContainsInternalRoute(
   return found
 }
 
-function getToolId(object: SyntaxNode): string | undefined {
+function resolveStaticStringExpression(
+  expression: SyntaxNode,
+  resolver: SelfHopResolver,
+  seen = new Set<string>()
+): string | undefined {
+  const value = unwrapExpression(expression)
+  const staticValue = getStaticString(value)
+  if (staticValue !== undefined) return staticValue
+  if (value.type !== 'Identifier' || typeof value.name !== 'string') return undefined
+  const key = `${resolver.file}:static-string:${value.name}`
+  if (seen.has(key)) return undefined
+  const binding = resolveScopedIdentifier(value.name, resolver)
+  if (!binding) return undefined
+  const nextSeen = new Set(seen)
+  nextSeen.add(key)
+  return resolveStaticStringExpression(binding.expression, binding.resolver, nextSeen)
+}
+
+function getToolId(object: SyntaxNode, resolver: SelfHopResolver): string | undefined {
   const idProperty = getObjectProperty(object, 'id')
   if (!idProperty || !isSyntaxNode(idProperty.value)) return undefined
-  const value = unwrapExpression(idProperty.value)
-  return value.type === 'StringLiteral' && typeof value.value === 'string' ? value.value : undefined
+  return resolveStaticStringExpression(idProperty.value, resolver)
 }
 
 interface ScopedExpression {
@@ -1531,11 +1549,15 @@ function getResolvedObjectProperties(
   const lastIndex = endIndex ?? properties.length - 1
   for (let index = lastIndex; index >= 0; index -= 1) {
     const property = properties[index]
-    if (
-      (property.type === 'ObjectProperty' || property.type === 'ObjectMethod') &&
-      getStaticPropertyName(property) === name
-    ) {
-      return { properties: [{ property, request }], complete: true }
+    if (property.type === 'ObjectProperty' || property.type === 'ObjectMethod') {
+      const propertyName = getStaticPropertyName(property)
+      if (propertyName === name) {
+        return { properties: [{ property, request }], complete: true }
+      }
+      if (property.computed === true && propertyName === undefined) {
+        const earlier = getResolvedObjectProperties(request, name, seen, index - 1)
+        return { properties: earlier.properties, complete: false }
+      }
     }
     if (property.type !== 'SpreadElement' || !isSyntaxNode(property.argument)) continue
     const key = `${request.resolver.file}:spread:${property.start ?? index}:${name}`
@@ -1582,8 +1604,10 @@ export function auditToolSelfHops(source: string, file = 'source.ts'): ToolSelfH
 
   const visit = (node: SyntaxNode) => {
     if (node.type === 'ObjectExpression') {
-      const toolId = getToolId(node)
-      if (toolId) {
+      const idProperty = getObjectProperty(node, 'id')
+      const toolId = idProperty ? getToolId(node, resolver) : undefined
+      const directRequestProperty = getObjectProperty(node, 'request')
+      if (idProperty && (toolId !== undefined || directRequestProperty)) {
         const toolObject: ResolvedRequestObject = {
           expression: node,
           resolver,