fix(security): block private/reserved IPs for hosted 1Password Connect SSRF (#4818)

* fix(security): block private/reserved IPs for hosted 1Password Connect SSRF * test(security): use real isPrivateOrReservedIP and cover IPv6 edge cases
2026-09-24 15:45:35 +08:00 · 2026-05-30 18:13:55 -07:00
parent a8f86c0c83
commit 911586ad5a
2 changed files with 156 additions and 20 deletions
@@ -0,0 +1,108 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockDnsLookup, hostedFlag } = vi.hoisted(() => ({
+  mockDnsLookup: vi.fn(),
+  hostedFlag: { value: false },
+}))
+
+vi.mock('@/lib/core/config/feature-flags', () => ({
+  get isHosted() {
+    return hostedFlag.value
+  },
+}))
+
+vi.mock('dns/promises', () => ({
+  default: { lookup: mockDnsLookup },
+}))
+
+import { validateConnectServerUrl } from '@/app/api/tools/onepassword/utils'
+
+describe('validateConnectServerUrl', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    hostedFlag.value = false
+  })
+
+  it('rejects a non-URL string', async () => {
+    await expect(validateConnectServerUrl('not a url')).rejects.toThrow('is not a valid URL')
+  })
+
+  describe('hosted deployment', () => {
+    beforeEach(() => {
+      hostedFlag.value = true
+    })
+
+    it.each([
+      ['loopback', 'http://127.0.0.1:8080'],
+      ['RFC1918 10.x', 'http://10.0.0.5'],
+      ['RFC1918 192.168.x', 'http://192.168.1.1:8443'],
+      ['RFC1918 172.16.x', 'http://172.16.0.9'],
+      ['link-local metadata', 'http://169.254.169.254'],
+      ['IPv4-mapped IPv6 private', 'http://[::ffff:10.0.0.1]'],
+      ['IPv6 loopback', 'http://[::1]'],
+    ])('blocks %s', async (_label, url) => {
+      await expect(validateConnectServerUrl(url)).rejects.toThrow(
+        'cannot point to a private or reserved IP address'
+      )
+    })
+
+    it('allows a public IP literal', async () => {
+      await expect(validateConnectServerUrl('https://8.8.8.8')).resolves.toBe('8.8.8.8')
+    })
+
+    it('blocks a hostname that resolves to a private IP', async () => {
+      mockDnsLookup.mockResolvedValue({ address: '10.1.2.3', family: 4 })
+      await expect(validateConnectServerUrl('https://connect.internal')).rejects.toThrow(
+        'cannot point to a private or reserved IP address'
+      )
+    })
+
+    it('allows a hostname that resolves to a public IP', async () => {
+      mockDnsLookup.mockResolvedValue({ address: '93.184.216.34', family: 4 })
+      await expect(validateConnectServerUrl('https://connect.example.com')).resolves.toBe(
+        '93.184.216.34'
+      )
+    })
+  })
+
+  describe('self-hosted deployment', () => {
+    beforeEach(() => {
+      hostedFlag.value = false
+    })
+
+    it.each([
+      ['loopback', 'http://127.0.0.1:8080', '127.0.0.1'],
+      ['RFC1918 10.x', 'http://10.0.0.5', '10.0.0.5'],
+      ['RFC1918 192.168.x', 'http://192.168.1.1:8443', '192.168.1.1'],
+    ])('allows %s (private Connect server)', async (_label, url, expected) => {
+      await expect(validateConnectServerUrl(url)).resolves.toBe(expected)
+    })
+
+    it('still blocks link-local metadata', async () => {
+      await expect(validateConnectServerUrl('http://169.254.169.254')).rejects.toThrow(
+        'cannot point to a link-local address'
+      )
+    })
+
+    it('still blocks IPv6 link-local', async () => {
+      await expect(validateConnectServerUrl('http://[fe80::1]')).rejects.toThrow(
+        'cannot point to a link-local address'
+      )
+    })
+
+    it('allows a hostname that resolves to a private IP', async () => {
+      mockDnsLookup.mockResolvedValue({ address: '10.1.2.3', family: 4 })
+      await expect(validateConnectServerUrl('https://connect.internal')).resolves.toBe('10.1.2.3')
+    })
+  })
+
+  it('rejects when DNS resolution fails', async () => {
+    mockDnsLookup.mockRejectedValue(new Error('ENOTFOUND'))
+    await expect(validateConnectServerUrl('https://nope.invalid')).rejects.toThrow(
+      'could not be resolved'
+    )
+  })
+})
@@ -12,7 +12,11 @@ import type {
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
 import * as ipaddr from 'ipaddr.js'
-import { secureFetchWithPinnedIP } from '@/lib/core/security/input-validation.server'
+import { isHosted } from '@/lib/core/config/feature-flags'
+import {
+  isPrivateOrReservedIP,
+  secureFetchWithPinnedIP,
+} from '@/lib/core/security/input-validation.server'

 /** Connect-format field type strings returned by normalization. */
 type ConnectFieldType =
@@ -246,12 +250,44 @@ export async function createOnePasswordClient(serviceAccountToken: string) {
 const connectLogger = createLogger('OnePasswordConnect')

 /**
- * Validates that a Connect server URL does not target cloud metadata endpoints.
- * Allows private IPs and localhost since 1Password Connect is designed to be self-hosted.
- * Returns the resolved IP for DNS pinning to prevent TOCTOU rebinding.
- * @throws Error if the URL is invalid, points to a link-local address, or DNS fails.
+ * Enforces the SSRF policy for a resolved Connect server IP.
+ *
+ * On the hosted service, all private and reserved IPs are blocked — a tenant has
+ * no legitimate reason to point Connect at the platform's internal network. On
+ * self-hosted deployments only link-local (cloud metadata) is blocked, since the
+ * operator controls both the workflows and the network and Connect servers
+ * legitimately live on private (RFC1918) addresses.
+ *
+ * @throws Error if the IP is not permitted under the active policy.
 */
-async function validateConnectServerUrl(serverUrl: string): Promise<string> {
+function assertConnectIpAllowed(ip: string, hostname: string): void {
+  if (isHosted) {
+    if (isPrivateOrReservedIP(ip)) {
+      connectLogger.warn('1Password Connect server URL resolves to a private or reserved IP', {
+        hostname,
+        resolvedIP: ip,
+      })
+      throw new Error('1Password server URL cannot point to a private or reserved IP address')
+    }
+    return
+  }
+
+  if (ipaddr.isValid(ip) && ipaddr.process(ip).range() === 'linkLocal') {
+    connectLogger.warn('1Password Connect server URL resolves to a link-local IP', {
+      hostname,
+      resolvedIP: ip,
+    })
+    throw new Error('1Password server URL cannot point to a link-local address')
+  }
+}
+
+/**
+ * Validates a Connect server URL against the SSRF policy and returns the resolved
+ * IP for DNS pinning to prevent TOCTOU rebinding. See {@link assertConnectIpAllowed}
+ * for the hosted vs. self-hosted policy.
+ * @throws Error if the URL is invalid, fails the IP policy, or DNS fails.
+ */
+export async function validateConnectServerUrl(serverUrl: string): Promise<string> {
  let hostname: string
  try {
    hostname = new URL(serverUrl).hostname
@@ -263,31 +299,23 @@ async function validateConnectServerUrl(serverUrl: string): Promise<string> {
    hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname

  if (ipaddr.isValid(clean)) {
-    const addr = ipaddr.process(clean)
-    if (addr.range() === 'linkLocal') {
-      throw new Error('1Password server URL cannot point to a link-local address')
-    }
+    assertConnectIpAllowed(clean, clean)
    return clean
  }

+  let address: string
  try {
-    const { address } = await dns.lookup(clean, { verbatim: true })
-    if (ipaddr.isValid(address) && ipaddr.process(address).range() === 'linkLocal') {
-      connectLogger.warn('1Password Connect server URL resolves to link-local IP', {
-        hostname: clean,
-        resolvedIP: address,
-      })
-      throw new Error('1Password server URL resolves to a link-local address')
-    }
-    return address
+    ;({ address } = await dns.lookup(clean, { verbatim: true }))
  } catch (error) {
-    if (error instanceof Error && error.message.startsWith('1Password')) throw error
    connectLogger.warn('DNS lookup failed for 1Password Connect server URL', {
      hostname: clean,
      error: toError(error).message,
    })
    throw new Error('1Password server URL hostname could not be resolved')
  }
+
+  assertConnectIpAllowed(address, clean)
+  return address
 }

 /** Minimal response shape used by all connectRequest callers. */