Skip to content

Commit a266eba

Browse files
committed
fix(desktop): fail closed on DNS resolution failure in the agent SSRF guard
checkAgentUrl now blocks when dns.lookup throws instead of allowing the load. Chromium resolves DNS independently of our Node lookup, so a host our resolver can't resolve could still reach a private address via Chromium's resolver — allowing it was a fail-open gap. Matches validateUrlWithDNS in apps/sim.
1 parent c1f9a84 commit a266eba

2 files changed

Lines changed: 13 additions & 8 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ describe('checkAgentUrl', () => {
5656
expect((await checkAgentUrl('https://mixed.test/')).ok).toBe(false)
5757
})
5858

59-
it('allows the load to proceed when DNS resolution fails', async () => {
59+
it('fails closed when DNS resolution fails', async () => {
6060
mockLookup.mockRejectedValue(new Error('ENOTFOUND'))
61-
expect((await checkAgentUrl('https://nope.invalid/')).ok).toBe(true)
61+
expect((await checkAgentUrl('https://nope.invalid/')).ok).toBe(false)
6262
})
6363
})
6464

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ const BLOCKED: UrlGuardResult = {
2626
* fetches and blocks any that land on a private/reserved address.
2727
*
2828
* IP literals are classified directly; hostnames are DNS-resolved and every
29-
* returned address is checked. A resolution failure is not treated as a block —
30-
* an unresolvable host reaches nothing, and Chromium fails the load naturally.
31-
* The residual DNS-rebinding TOCTOU window (our lookup vs Chromium's) is only
32-
* fully closable with egress firewalling; {@link isBlockedRequestUrl} adds a
33-
* synchronous per-request literal-IP backstop for redirects and subresources.
29+
* returned address is checked. Resolution failure fails CLOSED (blocks): we
30+
* can't confirm the host is public, and Chromium resolves independently, so it
31+
* could still reach a private address our lookup missed — matching
32+
* `validateUrlWithDNS` in `apps/sim`. The residual DNS-rebinding TOCTOU window
33+
* (our lookup vs Chromium's) is only fully closable with egress firewalling;
34+
* {@link isBlockedRequestUrl} adds a synchronous per-request literal-IP backstop
35+
* for redirects and subresources.
3436
*/
3537
export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
3638
const url = parseHttpUrl(rawUrl)
@@ -56,10 +58,13 @@ export async function checkAgentUrl(rawUrl: string): Promise<UrlGuardResult> {
5658
return BLOCKED
5759
}
5860
} catch (error) {
59-
logger.info('Agent navigation host did not resolve; letting the load fail naturally', {
61+
// Fail closed: an unresolved host can't be confirmed public, and Chromium
62+
// resolves independently, so it could still reach a private address.
63+
logger.warn('Agent navigation host did not resolve; blocking', {
6064
host,
6165
error: getErrorMessage(error),
6266
})
67+
return { ok: false, error: 'That address could not be resolved.' }
6368
}
6469

6570
return OK

0 commit comments

Comments
 (0)