Skip to content

fix(desktop): support session auth for env remote URLs - #66098

Open
nainaism wants to merge 3 commits into
NousResearch:mainfrom
nainaism:fix/desktop-env-remote-auth
Open

fix(desktop): support session auth for env remote URLs#66098
nainaism wants to merge 3 commits into
NousResearch:mainfrom
nainaism:fix/desktop-env-remote-auth

Conversation

@nainaism

@nainaism nainaism commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • allow HERMES_DESKTOP_REMOTE_URL to use a gateway's advertised session authentication when no static token is provided
  • preserve the existing static-token path when HERMES_DESKTOP_REMOTE_TOKEN is set
  • keep sign-in/sign-out controls available for environment-controlled gateways without overwriting the saved fallback connection
  • distinguish a first-time remote sign-in from an expired session in recovery copy and document both environment variables

Problem

Desktop currently treats HERMES_DESKTOP_REMOTE_URL as token-only and fails unless HERMES_DESKTOP_REMOTE_TOKEN is also set. That conflicts with Gateway settings and the environment-variable docs, which support session-based OAuth or username/password sign-in. It also lets a saved fallback token leak into the environment-controlled settings view, hiding the sign-in path.

Implementation

  • probe the gateway's public auth metadata for URL-only environment overrides
  • select session auth when the gateway advertises it; otherwise require a static token with a targeted error
  • prevent environment-controlled URLs from inheriting saved fallback tokens
  • return tokenless environment overrides immediately as initial session-auth UI state instead of blocking Settings on a main-process probe or inheriting stale saved auth
  • let the renderer's existing asynchronous probe refine the displayed mode while the connect path independently enforces the gateway's advertised auth mode
  • allow session sign-in without persisting the environment URL into connection.json
  • keep profile-specific overrides unchanged

Related work

Validation

  • targeted auth/settings tests — 70 passed
  • npm test — 1,781 passed, 1 skipped; 4 unrelated high-load UI failures in messaging/skills passed 5/5 when rerun in isolation
  • npm run typecheck — passed
  • npm run lint — passed (10 pre-existing warnings, 0 errors)
  • npm run build — passed
  • git diff --check — passed

Allow URL-only HERMES_DESKTOP_REMOTE_URL overrides to probe the gateway's advertised session auth while preserving the legacy static-token path. Keep sign-in controls available without overwriting the saved fallback connection, and document the updated behavior.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 17, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Looks Good

  • Clean enhancement: supports session auth for env remote URLs in desktop
  • Refactored effectiveRemoteToken helper cleanly handles the priority: envOverride > envToken > decrypted block.token
  • Updated documentation in HERMES_DESKTOP_REMOTE_URL table clarifies the session provider behavior
  • No security issues — token decryption path preserved for cases without env override

Reviewed by Hermes Agent

nainaism added 2 commits July 17, 2026 14:27
Keep tokenless environment-controlled gateways on the session sign-in UI when the public auth probe is temporarily unavailable, instead of inheriting an unrelated saved fallback mode.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

Supports session auth for env remote URLs in desktop (201 additions / 34 deletions). Prior COMMENT activity noted.

Desktop auth extension for remote URLs. No security issues. No hardcoded secrets.


Reviewed by Hermes Agent (cron batch 2026-07-17)

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing a real current-main inconsistency: apps/desktop/electron/main.ts:6129-6137 currently rejects a URL-only override, while website/docs/reference/environment-variables.md:508 says users can sign in from Gateway settings.

Problems

  • The new initial oauth mode still does not make first-time URL-only sign-in available immediately. The unchanged authResolved gate in apps/desktop/src/app/settings/gateway-settings.tsx:301-307 is false while the probe is pending or has errored when there is neither a saved token nor an existing OAuth cookie. Consequently the OAuth controls at :978-1009 remain hidden despite the new initial state.

Suggested changes

  • Make the renderer explicitly handle the tokenless environment candidate during probe-pending/error states, while letting a completed probe remain authoritative for token-only gateways. Cover that rendered control behavior rather than only the new pure helper.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants