Skip to content

Commit 39754e7

Browse files
committed
fix(desktop): allow browser agent localhost navigation
1 parent 055b7b4 commit 39754e7

3 files changed

Lines changed: 100 additions & 9 deletions

File tree

apps/desktop/src/main/browser-agent/session.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,39 @@ describe('reopening a closed tab', () => {
10851085
expect((reopened?.view as unknown as MockView).webContents.loadURL).not.toHaveBeenCalled()
10861086
})
10871087

1088+
it('duplicates a tab by loading the same URL in a new one', () => {
1089+
session.ensureTab()
1090+
const source = session.addTab()
1091+
;(source.view as unknown as MockView).webContents.getURL.mockReturnValue(
1092+
'https://example.com/inbox'
1093+
)
1094+
1095+
const copy = session.duplicateTab(source.id)
1096+
1097+
expect(copy?.id).not.toBe(source.id)
1098+
expect((copy?.view as unknown as MockView).webContents.loadURL).toHaveBeenCalledWith(
1099+
'https://example.com/inbox'
1100+
)
1101+
})
1102+
1103+
it('never copies a URL carrying embedded credentials into a duplicate', () => {
1104+
session.ensureTab()
1105+
const source = session.addTab()
1106+
;(source.view as unknown as MockView).webContents.getURL.mockReturnValue(
1107+
'https://user:pass@example.com/'
1108+
)
1109+
1110+
const copy = session.duplicateTab(source.id)
1111+
1112+
// Falls back to a blank tab rather than re-sending the credentials.
1113+
expect((copy?.view as unknown as MockView).webContents.loadURL).not.toHaveBeenCalled()
1114+
})
1115+
1116+
it('returns null when duplicating a tab that is not open', () => {
1117+
session.ensureTab()
1118+
expect(session.duplicateTab('no-such-tab')).toBeNull()
1119+
})
1120+
10881121
it('drops a non-http scheme from the reopen list', () => {
10891122
session.ensureTab()
10901123
const closing = session.addTab()

apps/desktop/src/main/browser-agent/url-guard.test.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,38 @@ describe('checkAgentUrl', () => {
2525
})
2626

2727
it('blocks private IP literals without resolving', async () => {
28-
expect((await checkAgentUrl('http://127.0.0.1/')).ok).toBe(false)
2928
expect((await checkAgentUrl('http://169.254.169.254/latest/meta-data')).ok).toBe(false)
3029
expect((await checkAgentUrl('http://10.0.0.5/')).ok).toBe(false)
31-
expect((await checkAgentUrl('http://[::1]/')).ok).toBe(false)
30+
expect((await checkAgentUrl('http://192.168.1.1/')).ok).toBe(false)
31+
expect((await checkAgentUrl('http://[fd00::1]/')).ok).toBe(false)
3232
expect(mockLookup).not.toHaveBeenCalled()
3333
})
3434

35+
it('allows loopback, so a local dev server can be opened', async () => {
36+
// The panel exists to browse from this machine, and the agent already has
37+
// an unrestricted shell on it. The LAN and the metadata endpoint above are
38+
// a different matter and stay blocked.
39+
expect((await checkAgentUrl('http://127.0.0.1:3000/')).ok).toBe(true)
40+
expect((await checkAgentUrl('http://[::1]:3000/')).ok).toBe(true)
41+
expect(mockLookup).not.toHaveBeenCalled()
42+
})
43+
44+
it('allows localhost, which resolves to loopback', async () => {
45+
mockLookup.mockResolvedValue([
46+
{ address: '::1', family: 6 },
47+
{ address: '127.0.0.1', family: 4 },
48+
])
49+
expect((await checkAgentUrl('http://localhost:3000/app')).ok).toBe(true)
50+
})
51+
52+
it('still blocks a host that resolves to the LAN alongside loopback', async () => {
53+
mockLookup.mockResolvedValue([
54+
{ address: '127.0.0.1', family: 4 },
55+
{ address: '192.168.0.9', family: 4 },
56+
])
57+
expect((await checkAgentUrl('http://sneaky.test/')).ok).toBe(false)
58+
})
59+
3560
it('allows public IP literals without resolving', async () => {
3661
expect((await checkAgentUrl('https://8.8.8.8/')).ok).toBe(true)
3762
expect(mockLookup).not.toHaveBeenCalled()
@@ -77,10 +102,15 @@ describe('checkAgentUrl', () => {
77102
describe('isBlockedRequestUrl', () => {
78103
it('blocks literal private/reserved hosts', () => {
79104
expect(isBlockedRequestUrl('http://169.254.169.254/latest/meta-data')).toBe(true)
80-
expect(isBlockedRequestUrl('http://127.0.0.1:8080/x')).toBe(true)
105+
expect(isBlockedRequestUrl('http://10.0.0.5/x')).toBe(true)
81106
expect(isBlockedRequestUrl('https://[fd00::1]/')).toBe(true)
82107
})
83108

109+
it('allows loopback subresources, so a local page can load its own assets', () => {
110+
expect(isBlockedRequestUrl('http://127.0.0.1:8080/app.js')).toBe(false)
111+
expect(isBlockedRequestUrl('http://[::1]:8080/app.css')).toBe(false)
112+
})
113+
84114
it('allows public literals and hostnames (classified at nav time)', () => {
85115
expect(isBlockedRequestUrl('https://8.8.8.8/')).toBe(false)
86116
expect(isBlockedRequestUrl('https://example.com/x')).toBe(false)

apps/desktop/src/main/browser-agent/url-guard.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import dns from 'node:dns/promises'
22
import { createLogger } from '@sim/logger'
3-
import { isIpLiteral, isPrivateIp, isPrivateIpHost, unwrapIpv6Brackets } from '@sim/security/ssrf'
3+
import {
4+
isIpLiteral,
5+
isLoopbackIp,
6+
isPrivateIp,
7+
isPrivateIpHost,
8+
unwrapIpv6Brackets,
9+
} from '@sim/security/ssrf'
410
import { getErrorMessage } from '@sim/utils/errors'
511
import { parseHttpUrl } from '@/main/navigation'
612

@@ -37,6 +43,25 @@ export interface UrlGuardResult {
3743
}
3844

3945
const OK: UrlGuardResult = { ok: true }
46+
47+
/**
48+
* Whether an address is off limits to the embedded browser.
49+
*
50+
* Loopback is deliberately allowed: it is the user's own machine, and opening
51+
* a dev server on localhost is one of the most ordinary things to do in this
52+
* panel — the URL bar already assumes `http://` for it. Nothing is given away
53+
* by it either, since the desktop app hands the same agent an unrestricted
54+
* shell on that machine, so a blocked `http://localhost:3000` is one
55+
* `curl http://localhost:3000` away regardless.
56+
*
57+
* Every other private range stays blocked. Those are a different matter: the
58+
* LAN is other people's machines, and `169.254.169.254` is link-local rather
59+
* than loopback, so the cloud-metadata endpoint this guard exists for is
60+
* unaffected.
61+
*/
62+
function isBlockedAddress(ip: string): boolean {
63+
return isPrivateIp(ip) && !isLoopbackIp(ip)
64+
}
4065
const BLOCKED: UrlGuardResult = {
4166
ok: false,
4267
error: 'That address points to a private or internal network and was blocked.',
@@ -48,7 +73,8 @@ const BLOCKED: UrlGuardResult = {
4873
* loopback/RFC1918/link-local host (e.g. the `169.254.169.254` cloud-metadata
4974
* endpoint) would let a page's contents be read back through the read/snapshot
5075
* tools. This resolves the host the same way `apps/sim` does for outbound
51-
* fetches and blocks any that land on a private/reserved address.
76+
* fetches and blocks any that land on a private/reserved address — except
77+
* loopback, which is allowed (see {@link isBlockedAddress}).
5278
*
5379
* IP literals are classified directly; hostnames are DNS-resolved and every
5480
* returned address is checked. Resolution failure fails CLOSED (blocks): we
@@ -69,7 +95,7 @@ export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
6995

7096
// IP literal: classify directly, no DNS lookup needed.
7197
if (isIpLiteral(host)) {
72-
if (isPrivateIp(host)) {
98+
if (isBlockedAddress(host)) {
7399
logger.warn('Blocked agent navigation to private IP literal', { host })
74100
return BLOCKED
75101
}
@@ -78,7 +104,7 @@ export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
78104

79105
try {
80106
const resolved = await resolveHost(host)
81-
if (resolved.some(({ address }) => isPrivateIp(address))) {
107+
if (resolved.some(({ address }) => isBlockedAddress(address))) {
82108
logger.warn('Blocked agent navigation resolving to private IP', { host })
83109
return BLOCKED
84110
}
@@ -105,8 +131,10 @@ export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
105131
*/
106132
export function isBlockedRequestUrl(rawUrl: string): boolean {
107133
try {
108-
// isPrivateIpHost strips IPv6 brackets itself.
109-
return isPrivateIpHost(new URL(rawUrl).hostname)
134+
// isPrivateIpHost strips IPv6 brackets itself; unwrap again for the
135+
// loopback carve-out, which takes a bare address.
136+
const host = new URL(rawUrl).hostname
137+
return isPrivateIpHost(host) && !isLoopbackIp(unwrapIpv6Brackets(host))
110138
} catch {
111139
return false
112140
}

0 commit comments

Comments
 (0)