fix(desktop): harden link-title previews against SSRF by HengYangDS · Pull Request #65613 · NousResearch/hermes-agent

HengYangDS · 2026-07-16T11:56:46Z

Current change

Current head: 8a8c5dcb — rebased on NousResearch/main e598cef.

Desktop link-title previews now admit public HTTP(S) targets and redirect hops before a fetch, pin admitted DNS results into the SOCKS/curl transport, and fail closed when that transport cannot be preserved. The follow-up removes the hidden BrowserWindow renderer fallback entirely: an admitted URL is never renderer-navigated.

Root cause and security boundary

Automatic title resolution had multiple network paths. URL-shape checks alone did not consistently protect against redirect hops, DNS rebinding, proxy-environment bypass, or special-purpose/private addresses. The renderer fallback also created a separate native-browser surface that could trigger credential/passkey UI.

Review disposition

The renderer/passkey and unregistered-E2E concerns were addressed by deleting the renderer queue/window/E2E path rather than adding another suppression layer. Resolution is now curl-only on the pinned SOCKS path; a curl miss returns no title rather than navigating a renderer.

Validation

Node 22:

focused Electron link-title suite: 45 files, 451 passed, 1 skipped;
full Electron project: 48 files, 476 passed, 1 skipped;
UI suite: 193 files, 1574 passed, 1 skipped;
typecheck, lint (existing warnings only), Prettier, and build.

Review focus

Please review the boundary that no link-title path can renderer-navigate an admitted URL, and that redirect/DNS admission remains pinned to the curl/SOCKS transport.

alt-glitch · 2026-07-16T12:00:37Z

This was generated by AI during triage.

Related to #63171: both address automatic Desktop link-title preview SSRF risk, but this implementation retains the renderer fallback behind a pinned SOCKS transport while #63171 removes that fallback. Maintainers should compare approaches.

tonydwb

Code Review Summary

Verdict: Comment

Scope

18 files, +2658/-151 lines. Hardens link-title previews against SSRF.

Quality

Substantial security hardening for link-title previews.
Multi-platform impact (desktop + web).
High file count warrants careful security review.

Notes

SSRF hardening is security-sensitive — recommend human reviewer verify the URL-fetch isolation is complete.

Looks Good

Security-focused fix with significant scope.

Reviewed by Hermes Agent

teknium1

Thanks for the substantial DNS-pinning and transport hardening. The SSRF premise is confirmed on current main: apps/desktop/electron/main.ts:3955-3977 follows redirects with curl, and :4102-4107 loads the fallback URL in a hidden renderer.

Problems

The PR still loads arbitrary admitted public pages in apps/desktop/electron/main.ts:4298. apps/desktop/electron/link-title-window.ts:34-35 blocks popups/WebRTC, but does not address credential UI. The related #63171 discussion documents a Windows 11 reproduction where this renderer path caused a native passkey dialog for docs.google.com. Please remove the renderer fallback or add a tested session-level suppression for that UI before navigation.
apps/desktop/electron/link-title-window.e2e.ts:10 is not run by the configured test projects: apps/desktop/vitest.config.ts:19 only includes electron/**/*.test.ts, and no runner references the E2E file or its required receipt environment variable. Wire it into a deterministic Electron test command.

This is an automated hermes-sweeper review.

teknium1 · 2026-07-18T20:25:06Z

@@ -26,6 +30,19 @@ export function linkTitleWindowOptions(partitionSession) {
 export function createLinkTitleWindow(BrowserWindow, partitionSession) {
  const window = new BrowserWindow(linkTitleWindowOptions(partitionSession))

+  try {
+    window.webContents.setWindowOpenHandler(() => ({ action: 'deny' }))
+    window.webContents.setWebRTCIPHandlingPolicy('disable_non_proxied_udp')


This still leaves arbitrary admitted public pages running in the hidden renderer. The related #63171 discussion reports a Windows passkey dialog triggered by this exact fallback path; WebRTC/popup controls do not cover that. Please remove the fallback or add and test a title-session policy that suppresses credential/WebAuthn UI before navigation.

teknium1 · 2026-07-18T20:25:06Z

+import { startLinkTitleSocksGateway } from './link-title-socks'
+import { configureLinkTitleSession, createLinkTitleWindow, readLinkTitleWindowTitle } from './link-title-window'
+
+const receiptPath = process.env.HERMES_LINK_TITLE_E2E_RECEIPT


This program is not selected by apps/desktop/vitest.config.ts (electron/**/*.test.ts only), and no repository runner references this file or sets this required variable. Please wire it into a deterministic Electron test command so the proxy fail-closed assertion runs in CI.

HengYangDS · 2026-07-19T06:24:33Z

Addressed in 8a8c5dcb (rebased onto current upstream e598cef).

Removed the hidden BrowserWindow renderer fallback entirely: link-title resolution now remains on the pinned SOCKS/curl path, so an admitted page is never renderer-navigated and cannot trigger native credential/passkey UI. Removed the dead renderer queue/window/E2E code and added curl-only regression coverage.

Validated with Node 22: focused Electron 45 files / 451 passed / 1 skipped; full Electron 48 files / 476 passed / 1 skipped; UI 193 files / 1574 passed / 1 skipped; typecheck; lint (existing warnings only); Prettier; build.

HengYangDS · 2026-07-19T07:58:35Z

Maintainer-ready at 8a8c5dcb on e598cef.

CI for this fork PR is currently action_required and has not started jobs; if a maintainer can approve the fork workflow, I will address any resulting failure. The requested review focus is the curl-only, no-renderer-navigation boundary for link-title resolution.

HengYangDS · 2026-07-28T14:08:04Z

Rebased to current upstream main 9e2f07e704d6433c118cdb4543e8519e1fa62762.

New head: aebcb4b5eb46dab28167801eef93db7750e2e6ac.

Fresh Node 22 evidence on this exact base:

focused link-title transport/admission tests: 27 passed;
Desktop typecheck;
complete Electron project: 75 files passed, 1 skipped; 852 tests passed, 2 skipped;
complete UI project: 322 files, 2806 passed;
production build bound to the clean head.

Review boundary: automatic link-title preview stays on the admitted DNS-pinned curl/SOCKS path. The hidden BrowserWindow fallback was removed, so an admitted page is never renderer-navigated and cannot surface native credential/passkey UI through this feature.

The fork workflow remains action_required with no check run; a maintainer approval is still needed for GitHub Actions to execute.

(cherry picked from commit 8f453576afaca7660857643e29952c2e88e507d8)

(cherry picked from commit 911117d3f116e11ba485e2d5154967e4858643f0)

(cherry picked from commit 97e65daf946d7473fb7e1417112db01d12f6c2bb)

(cherry picked from commit bf17aff2b28ec3ea47ddb615d90e8d207dff262f)

(cherry picked from commit 5a6995a605102336709ce1dadecbb094896e632c)

(cherry picked from commit 42cfe35cea18db1c1d9fc5db5d77b1497967b75a)

(cherry picked from commit d5195829636c215a19c5985bd2b33cecabe8f0e6)

(cherry picked from commit 97ec0961da783797a27fbcfe9a97f9fd33baea61)

(cherry picked from commit 20f88a3422f52315417a95be2242a15a469a7272)

HengYangDS · 2026-07-29T09:38:33Z

Rebased and force-with-lease updated to current NousResearch/main:

base: 015718066ab8e9499c3caea3cda9f7ea469036fc
head: e8266b1a8f69d611b3e09c3852ed20d88fcc6c50

Fresh evidence on that exact head:

typecheck;
Electron suite: 77 files, 885 passed, 2 skipped;
UI suite: 332 files, 2,953 passed;
production build;
fork-only self-hosted validation passed, including the workflow's exact-checkout assertion (run 30440077248).

Security boundary unchanged: link-title preview uses the admitted DNS-pinned curl/SOCKS path only; the hidden renderer fallback and its test file are absent, so a curl miss returns no title rather than navigating a renderer.

The fork-only run is supplemental evidence; the upstream action_required workflow still needs maintainer approval before its own checks can run.

HengYangDS · 2026-07-29T16:20:30Z

Closing at author request: I am standardizing my local Hermes use on the official Homebrew Desktop/CLI path and will no longer maintain this fork-based contribution lane. The reviewed security work remains preserved in the PR history; maintainers may reuse it if useful. Thank you.

tonydwb reviewed Jul 16, 2026

View reviewed changes

alt-glitch mentioned this pull request Jul 17, 2026

fix(desktop): harden link title transport #66459

Closed

HengYangDS marked this pull request as ready for review July 18, 2026 05:07

teknium1 reviewed Jul 18, 2026

View reviewed changes

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch from 8a65ce1 to 8a8c5dc Compare July 19, 2026 06:17

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch 14 times, most recently from 6a3834c to f63dc4e Compare July 21, 2026 05:32

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch from f63dc4e to fa105d1 Compare July 21, 2026 07:38

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch 2 times, most recently from 18344ef to aebcb4b Compare July 28, 2026 14:04

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch 2 times, most recently from 7250557 to 0a94bd4 Compare July 29, 2026 08:10

HengYangDS added 10 commits July 29, 2026 17:27

fix(desktop): block local link title fetches

ed90541

(cherry picked from commit 8f453576afaca7660857643e29952c2e88e507d8)

fix(desktop): preserve public link title targets

de943d5

(cherry picked from commit 911117d3f116e11ba485e2d5154967e4858643f0)

fix(desktop): default deny non-public IPv6 titles

367516e

(cherry picked from commit 97e65daf946d7473fb7e1417112db01d12f6c2bb)

fix(desktop): bind link title fetches to admitted URLs

5ec996a

(cherry picked from commit bf17aff2b28ec3ea47ddb615d90e8d207dff262f)

fix(desktop): ignore curl default config

b5d0267

(cherry picked from commit 5a6995a605102336709ce1dadecbb094896e632c)

fix(desktop): classify resolved link title addresses

cbfe6b0

(cherry picked from commit 42cfe35cea18db1c1d9fc5db5d77b1497967b75a)

fix(desktop): pin link title DNS through local socks

4460dfa

(cherry picked from commit d5195829636c215a19c5985bd2b33cecabe8f0e6)

fix(desktop): bound pinned link title lifecycles

bc50fe1

(cherry picked from commit 97ec0961da783797a27fbcfe9a97f9fd33baea61)

fix(desktop): close pinned title transport gaps

86b1be2

(cherry picked from commit 20f88a3422f52315417a95be2242a15a469a7272)

fix(desktop): remove renderer title fallback

e8266b1

HengYangDS force-pushed the fix/link-title-url-admission-upstream-20260716 branch from 0a94bd4 to e8266b1 Compare July 29, 2026 09:32

HengYangDS closed this Jul 29, 2026

GottZ mentioned this pull request Jul 31, 2026

fix(desktop): prevent SSRF in automatic link-title previews #63171

Open

andrexibiza mentioned this pull request Sep 1, 2026

feat(desktop): click-to-expand link previews with SSRF-guarded resolver #99980

Open

Navigation Menu

fix(desktop): harden link-title previews against SSRF - #65613

fix(desktop): harden link-title previews against SSRF#65613
HengYangDS wants to merge 10 commits into
NousResearch:mainfrom
HengYangDS:fix/link-title-url-admission-upstream-20260716

HengYangDS commented Jul 16, 2026 •

edited

Loading

Uh oh!

alt-glitch commented Jul 16, 2026

Uh oh!

tonydwb left a comment

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 18, 2026

Uh oh!

teknium1 Jul 18, 2026

Uh oh!

HengYangDS commented Jul 19, 2026

Uh oh!

HengYangDS commented Jul 19, 2026

Uh oh!

HengYangDS commented Jul 28, 2026

Uh oh!

HengYangDS commented Jul 29, 2026

Uh oh!

HengYangDS commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

HengYangDS commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current change

Root cause and security boundary

Review disposition

Validation

Review focus

Uh oh!

alt-glitch commented Jul 16, 2026

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Code Review Summary

Scope

Quality

Notes

Looks Good

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Uh oh!

teknium1 Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

HengYangDS commented Jul 19, 2026

Uh oh!

HengYangDS commented Jul 19, 2026

Uh oh!

HengYangDS commented Jul 28, 2026

Uh oh!

HengYangDS commented Jul 29, 2026

Uh oh!

HengYangDS commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HengYangDS commented Jul 16, 2026 •

edited

Loading