Skip to content

fix(desktop): explain why SSH connection reuse silently stops persisting - #84913

Open
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/desktop-ssh-token-plaintext-opt-in
Open

fix(desktop): explain why SSH connection reuse silently stops persisting#84913
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/desktop-ssh-token-plaintext-opt-in

Conversation

@pierrenode

Copy link
Copy Markdown
Contributor

Summary

On keyring-less Linux (Hyprland/Sway, no GNOME Keyring or KWallet — the same environment #62319/salvaged-in-this-window fixed for the "remote" connection mode), persistSshConnectionToken() in apps/desktop/electron/main.ts calls encryptDesktopSecret(token) with no allowPlainText option:

function persistSshConnectionToken(profile, source, token) {
  try {
    const config = readDesktopConnectionConfig()
    const encrypted = encryptDesktopSecret(token)   // no options -> allowPlainText defaults false
    ...
  } catch (error: any) {
    sshRememberLog(`[ssh] could not persist served token: ${error.message}`)   // internal log only
  }
}

encryptDesktopSecret throws when safeStorage.isEncryptionAvailable() is false and allowPlainText isn't set. The catch only writes to the internal sshRememberLog diagnostic buffer — nothing surfaces to the UI. Every subsequent app launch re-spawns the remote connection instead of reusing the last one, with zero indication of why.

Why this stayed out of #62319's scope

The "remote" mode fix added allowPlainText as an explicit, per-action opt-in gated behind a destructive confirm dialog in Settings → Gateway, triggered from the discrete "Save"/"Apply" button click where the user just typed a token. SSH mode has no equivalent moment: buildSshBlock() never accepts a user-typed token at all — the only place an SSH token is ever set is persistSshConnectionToken(), called automatically after a successful SSH bootstrap. There's no "Save" click to gate a confirm dialog on, and prompting a blocking dialog from an automatic background reconnect would be poor UX (and might not even have a foreground window in every trigger path).

Given that, I kept this fix conservative: it does not change what gets persisted (SSH tokens still correctly refuse to persist without a keyring, same security posture as before). It fixes specifically the silent part of the bug — the reason a working feature quietly stops working with no UI trace.

Fix

sanitizeDesktopConnectionConfig() already computes secureTokenStorage (from safeStorage.isEncryptionAvailable()) mode-agnostically and sends it to the renderer for every connection mode — the "remote" mode banner already consumes it, gated to mode === 'remote' && authMode === 'token'. SSH mode never read this existing signal.

Adds an informational (non-destructive-styled, distinct from the existing plain-text-storage warning) notice in the SSH connection fields section of Settings → Gateway, shown when secureTokenStorage === false, explaining that connection reuse is unavailable on this machine and reuse can be restored by enabling GNOME Keyring or KWallet. No main.ts/hardening.ts changes — purely renderer + i18n.

Localized in en/ja/zh/zh-hant, matching the locale coverage of the sibling remote-mode banner from #62319. ar.ts has no strings at all for this settings section already (falls back to the existing locale→en runtime fallback), so it's left untouched for consistency with that precedent.

Testing

  • Two new tests in gateway-settings.test.tsx: the notice renders when mode: 'ssh', secureTokenStorage: false, and does not render when secureTokenStorage: true.
  • Mutation-verify: reverted the renderer change via git stash — the new "warns" test fails against the pre-fix code, passes after.
  • tsc --noEmit -p tsconfig.json: clean.
  • vitest run on gateway-settings.test.tsx + i18n/languages.test.ts + i18n/runtime.test.ts + boot-failure-overlay.test.tsx + boot-failure-reauth.test.ts (the two files most likely to reuse <GatewaySettings> in embedded mode): all pass (41 tests).
  • eslint could not run in this environment (root eslint.config.mjs needs a globals peer dependency not present in this workspace-scoped install) — relied on tsc --noEmit for type-safety instead.

Checklist

On keyring-less Linux (Hyprland/Sway, no GNOME Keyring or KWallet),
persistSshConnectionToken() calls encryptDesktopSecret(token) with no
allowPlainText option — it throws, the catch logs only to the internal
sshRememberLog buffer, and every future launch re-spawns the remote
connection instead of reusing the one from last time, with no indication
in the UI of why.

The sibling "remote" mode connection path already reports this exact
signal (secureTokenStorage, computed from safeStorage.isEncryptionAvailable()
in sanitizeDesktopConnectionConfig — mode-agnostic, not remote-specific)
and surfaces it via a persistent banner in Settings -> Gateway. SSH mode
never read it: the notice was gated to remote/token auth only.

This does not change persistence behavior — the SSH-issued dashboard token
still refuses to persist without a keyring, matching the pre-existing
security posture (unlike remote mode's user-typed credential, the SSH
token isn't collected through a discrete "Save" action a confirm dialog
could gate, so this stays out of scope for now; see PR body). It surfaces
the *reason* connection reuse silently degrades, reusing the exact
secureTokenStorage signal already computed and already sent to the
renderer for every connection mode.

Adds an informational notice under the SSH connection fields, gated on
the existing secureTokenStorage flag, localized in en/ja/zh/zh-hant
(matching the sibling banner's locale coverage; ar.ts has no strings for
this settings section and relies on the existing locale->en fallback).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 13, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(desktop): explain why SSH connection reuse silently stops persisting

No blocking issues found. A few minor observations:

  1. Two banners on the same signal. The new warning and the existing remote-mode plain-text banner both key off secureTokenStorage === false (as the code comment notes), so in the common case users will see two stacked alerts about the same root cause. Consider merging, or scoping the new one strictly to the reuse aspect so it doesn't render redundantly next to the storage banner.
  2. Linux-centric remediation copy. "Install or enable GNOME Keyring or KWallet" is wrong/unhelpful on macOS (Keychain) and Windows (Credential Manager). Platform-aware wording — or a generic "enable your OS keyring" — would fit every host.
  3. i18n coverage. ar.ts was not updated; the runtime falls back to en for missing keys, so nothing breaks and partial locales are established practice — but a one-line addition would keep Arabic users from seeing English in an otherwise translated section.
  4. Tests cover both banner-present and banner-absent cases — good. Minor: confirm state.secureTokenStorage is derived from the connection config on every entry path into the SSH panel, not just the one exercised by the tests.

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 comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have 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.

3 participants