Skip to content

Commit 5d4563a

Browse files
committed
fix(security): block private/reserved IPs for hosted 1Password Connect SSRF
1 parent a8f86c0 commit 5d4563a

2 files changed

Lines changed: 164 additions & 20 deletions

File tree

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { inputValidationMock, inputValidationMockFns } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mockDnsLookup, hostedFlag } = vi.hoisted(() => ({
8+
mockDnsLookup: vi.fn(),
9+
hostedFlag: { value: false },
10+
}))
11+
12+
vi.mock('@/lib/core/config/feature-flags', () => ({
13+
get isHosted() {
14+
return hostedFlag.value
15+
},
16+
}))
17+
18+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
19+
20+
vi.mock('dns/promises', () => ({
21+
default: { lookup: mockDnsLookup },
22+
}))
23+
24+
import { validateConnectServerUrl } from '@/app/api/tools/onepassword/utils'
25+
26+
/** Mirrors the real isPrivateOrReservedIP for the ranges exercised here. */
27+
function fakeIsPrivateOrReservedIP(ip: string): boolean {
28+
if (ip.startsWith('10.') || ip.startsWith('192.168.')) return true
29+
if (ip.startsWith('172.')) {
30+
const second = Number.parseInt(ip.split('.')[1], 10)
31+
if (second >= 16 && second <= 31) return true
32+
}
33+
if (ip.startsWith('169.254.')) return true
34+
if (ip.startsWith('127.') || ip === '::1') return true
35+
return false
36+
}
37+
38+
describe('validateConnectServerUrl', () => {
39+
beforeEach(() => {
40+
vi.clearAllMocks()
41+
hostedFlag.value = false
42+
inputValidationMockFns.mockIsPrivateOrReservedIP.mockImplementation(fakeIsPrivateOrReservedIP)
43+
})
44+
45+
it('rejects a non-URL string', async () => {
46+
await expect(validateConnectServerUrl('not a url')).rejects.toThrow('is not a valid URL')
47+
})
48+
49+
describe('hosted deployment', () => {
50+
beforeEach(() => {
51+
hostedFlag.value = true
52+
})
53+
54+
it.each([
55+
['loopback', 'http://127.0.0.1:8080'],
56+
['RFC1918 10.x', 'http://10.0.0.5'],
57+
['RFC1918 192.168.x', 'http://192.168.1.1:8443'],
58+
['RFC1918 172.16.x', 'http://172.16.0.9'],
59+
['link-local metadata', 'http://169.254.169.254'],
60+
])('blocks %s', async (_label, url) => {
61+
await expect(validateConnectServerUrl(url)).rejects.toThrow(
62+
'cannot point to a private or reserved IP address'
63+
)
64+
})
65+
66+
it('allows a public IP literal', async () => {
67+
await expect(validateConnectServerUrl('https://8.8.8.8')).resolves.toBe('8.8.8.8')
68+
})
69+
70+
it('blocks a hostname that resolves to a private IP', async () => {
71+
mockDnsLookup.mockResolvedValue({ address: '10.1.2.3', family: 4 })
72+
await expect(validateConnectServerUrl('https://connect.internal')).rejects.toThrow(
73+
'cannot point to a private or reserved IP address'
74+
)
75+
})
76+
77+
it('allows a hostname that resolves to a public IP', async () => {
78+
mockDnsLookup.mockResolvedValue({ address: '93.184.216.34', family: 4 })
79+
await expect(validateConnectServerUrl('https://connect.example.com')).resolves.toBe(
80+
'93.184.216.34'
81+
)
82+
})
83+
})
84+
85+
describe('self-hosted deployment', () => {
86+
beforeEach(() => {
87+
hostedFlag.value = false
88+
})
89+
90+
it.each([
91+
['loopback', 'http://127.0.0.1:8080', '127.0.0.1'],
92+
['RFC1918 10.x', 'http://10.0.0.5', '10.0.0.5'],
93+
['RFC1918 192.168.x', 'http://192.168.1.1:8443', '192.168.1.1'],
94+
])('allows %s (private Connect server)', async (_label, url, expected) => {
95+
await expect(validateConnectServerUrl(url)).resolves.toBe(expected)
96+
})
97+
98+
it('still blocks link-local metadata', async () => {
99+
await expect(validateConnectServerUrl('http://169.254.169.254')).rejects.toThrow(
100+
'cannot point to a link-local address'
101+
)
102+
})
103+
104+
it('allows a hostname that resolves to a private IP', async () => {
105+
mockDnsLookup.mockResolvedValue({ address: '10.1.2.3', family: 4 })
106+
await expect(validateConnectServerUrl('https://connect.internal')).resolves.toBe('10.1.2.3')
107+
})
108+
})
109+
110+
it('rejects when DNS resolution fails', async () => {
111+
mockDnsLookup.mockRejectedValue(new Error('ENOTFOUND'))
112+
await expect(validateConnectServerUrl('https://nope.invalid')).rejects.toThrow(
113+
'could not be resolved'
114+
)
115+
})
116+
})

apps/sim/app/api/tools/onepassword/utils.ts

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import type {
1212
import { createLogger } from '@sim/logger'
1313
import { toError } from '@sim/utils/errors'
1414
import * as ipaddr from 'ipaddr.js'
15-
import { secureFetchWithPinnedIP } from '@/lib/core/security/input-validation.server'
15+
import { isHosted } from '@/lib/core/config/feature-flags'
16+
import {
17+
isPrivateOrReservedIP,
18+
secureFetchWithPinnedIP,
19+
} from '@/lib/core/security/input-validation.server'
1620

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

248252
/**
249-
* Validates that a Connect server URL does not target cloud metadata endpoints.
250-
* Allows private IPs and localhost since 1Password Connect is designed to be self-hosted.
251-
* Returns the resolved IP for DNS pinning to prevent TOCTOU rebinding.
252-
* @throws Error if the URL is invalid, points to a link-local address, or DNS fails.
253+
* Enforces the SSRF policy for a resolved Connect server IP.
254+
*
255+
* On the hosted service, all private and reserved IPs are blocked — a tenant has
256+
* no legitimate reason to point Connect at the platform's internal network. On
257+
* self-hosted deployments only link-local (cloud metadata) is blocked, since the
258+
* operator controls both the workflows and the network and Connect servers
259+
* legitimately live on private (RFC1918) addresses.
260+
*
261+
* @throws Error if the IP is not permitted under the active policy.
253262
*/
254-
async function validateConnectServerUrl(serverUrl: string): Promise<string> {
263+
function assertConnectIpAllowed(ip: string, hostname: string): void {
264+
if (isHosted) {
265+
if (isPrivateOrReservedIP(ip)) {
266+
connectLogger.warn('1Password Connect server URL resolves to a private or reserved IP', {
267+
hostname,
268+
resolvedIP: ip,
269+
})
270+
throw new Error('1Password server URL cannot point to a private or reserved IP address')
271+
}
272+
return
273+
}
274+
275+
if (ipaddr.isValid(ip) && ipaddr.process(ip).range() === 'linkLocal') {
276+
connectLogger.warn('1Password Connect server URL resolves to a link-local IP', {
277+
hostname,
278+
resolvedIP: ip,
279+
})
280+
throw new Error('1Password server URL cannot point to a link-local address')
281+
}
282+
}
283+
284+
/**
285+
* Validates a Connect server URL against the SSRF policy and returns the resolved
286+
* IP for DNS pinning to prevent TOCTOU rebinding. See {@link assertConnectIpAllowed}
287+
* for the hosted vs. self-hosted policy.
288+
* @throws Error if the URL is invalid, fails the IP policy, or DNS fails.
289+
*/
290+
export async function validateConnectServerUrl(serverUrl: string): Promise<string> {
255291
let hostname: string
256292
try {
257293
hostname = new URL(serverUrl).hostname
@@ -263,31 +299,23 @@ async function validateConnectServerUrl(serverUrl: string): Promise<string> {
263299
hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname
264300

265301
if (ipaddr.isValid(clean)) {
266-
const addr = ipaddr.process(clean)
267-
if (addr.range() === 'linkLocal') {
268-
throw new Error('1Password server URL cannot point to a link-local address')
269-
}
302+
assertConnectIpAllowed(clean, clean)
270303
return clean
271304
}
272305

306+
let address: string
273307
try {
274-
const { address } = await dns.lookup(clean, { verbatim: true })
275-
if (ipaddr.isValid(address) && ipaddr.process(address).range() === 'linkLocal') {
276-
connectLogger.warn('1Password Connect server URL resolves to link-local IP', {
277-
hostname: clean,
278-
resolvedIP: address,
279-
})
280-
throw new Error('1Password server URL resolves to a link-local address')
281-
}
282-
return address
308+
;({ address } = await dns.lookup(clean, { verbatim: true }))
283309
} catch (error) {
284-
if (error instanceof Error && error.message.startsWith('1Password')) throw error
285310
connectLogger.warn('DNS lookup failed for 1Password Connect server URL', {
286311
hostname: clean,
287312
error: toError(error).message,
288313
})
289314
throw new Error('1Password server URL hostname could not be resolved')
290315
}
316+
317+
assertConnectIpAllowed(address, clean)
318+
return address
291319
}
292320

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

0 commit comments

Comments
 (0)