From 5d4563a7ad07d767eb4c3b76f82a21cfa63fa2f4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 30 May 2026 18:02:20 -0700 Subject: [PATCH 1/2] fix(security): block private/reserved IPs for hosted 1Password Connect SSRF --- .../app/api/tools/onepassword/utils.test.ts | 116 ++++++++++++++++++ apps/sim/app/api/tools/onepassword/utils.ts | 68 +++++++--- 2 files changed, 164 insertions(+), 20 deletions(-) create mode 100644 apps/sim/app/api/tools/onepassword/utils.test.ts diff --git a/apps/sim/app/api/tools/onepassword/utils.test.ts b/apps/sim/app/api/tools/onepassword/utils.test.ts new file mode 100644 index 0000000000..73b1937cab --- /dev/null +++ b/apps/sim/app/api/tools/onepassword/utils.test.ts @@ -0,0 +1,116 @@ +/** + * @vitest-environment node + */ +import { inputValidationMock, inputValidationMockFns } from '@sim/testing' +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('@/lib/core/security/input-validation.server', () => inputValidationMock) + +vi.mock('dns/promises', () => ({ + default: { lookup: mockDnsLookup }, +})) + +import { validateConnectServerUrl } from '@/app/api/tools/onepassword/utils' + +/** Mirrors the real isPrivateOrReservedIP for the ranges exercised here. */ +function fakeIsPrivateOrReservedIP(ip: string): boolean { + if (ip.startsWith('10.') || ip.startsWith('192.168.')) return true + if (ip.startsWith('172.')) { + const second = Number.parseInt(ip.split('.')[1], 10) + if (second >= 16 && second <= 31) return true + } + if (ip.startsWith('169.254.')) return true + if (ip.startsWith('127.') || ip === '::1') return true + return false +} + +describe('validateConnectServerUrl', () => { + beforeEach(() => { + vi.clearAllMocks() + hostedFlag.value = false + inputValidationMockFns.mockIsPrivateOrReservedIP.mockImplementation(fakeIsPrivateOrReservedIP) + }) + + 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'], + ])('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('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' + ) + }) +}) diff --git a/apps/sim/app/api/tools/onepassword/utils.ts b/apps/sim/app/api/tools/onepassword/utils.ts index 94babba28f..4dcee71696 100644 --- a/apps/sim/app/api/tools/onepassword/utils.ts +++ b/apps/sim/app/api/tools/onepassword/utils.ts @@ -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 { +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 { let hostname: string try { hostname = new URL(serverUrl).hostname @@ -263,31 +299,23 @@ async function validateConnectServerUrl(serverUrl: string): Promise { 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. */ From 243a06eef25d4518d762d918d7c98f9e48f819d8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 30 May 2026 18:09:10 -0700 Subject: [PATCH 2/2] test(security): use real isPrivateOrReservedIP and cover IPv6 edge cases --- .../app/api/tools/onepassword/utils.test.ts | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/apps/sim/app/api/tools/onepassword/utils.test.ts b/apps/sim/app/api/tools/onepassword/utils.test.ts index 73b1937cab..504ed46b05 100644 --- a/apps/sim/app/api/tools/onepassword/utils.test.ts +++ b/apps/sim/app/api/tools/onepassword/utils.test.ts @@ -1,7 +1,6 @@ /** * @vitest-environment node */ -import { inputValidationMock, inputValidationMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockDnsLookup, hostedFlag } = vi.hoisted(() => ({ @@ -15,31 +14,16 @@ vi.mock('@/lib/core/config/feature-flags', () => ({ }, })) -vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) - vi.mock('dns/promises', () => ({ default: { lookup: mockDnsLookup }, })) import { validateConnectServerUrl } from '@/app/api/tools/onepassword/utils' -/** Mirrors the real isPrivateOrReservedIP for the ranges exercised here. */ -function fakeIsPrivateOrReservedIP(ip: string): boolean { - if (ip.startsWith('10.') || ip.startsWith('192.168.')) return true - if (ip.startsWith('172.')) { - const second = Number.parseInt(ip.split('.')[1], 10) - if (second >= 16 && second <= 31) return true - } - if (ip.startsWith('169.254.')) return true - if (ip.startsWith('127.') || ip === '::1') return true - return false -} - describe('validateConnectServerUrl', () => { beforeEach(() => { vi.clearAllMocks() hostedFlag.value = false - inputValidationMockFns.mockIsPrivateOrReservedIP.mockImplementation(fakeIsPrivateOrReservedIP) }) it('rejects a non-URL string', async () => { @@ -57,6 +41,8 @@ describe('validateConnectServerUrl', () => { ['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' @@ -101,6 +87,12 @@ describe('validateConnectServerUrl', () => { ) }) + 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')