Skip to content

feat(desktop): auto-detect Linux keychain backend for secure token storage - #41236

Merged
3 commits merged into
NousResearch:mainfrom
hsearcy:linux-keychain-auto-detect
Aug 13, 2026
Merged

feat(desktop): auto-detect Linux keychain backend for secure token storage#41236
3 commits merged into
NousResearch:mainfrom
hsearcy:linux-keychain-auto-detect

Conversation

@hsearcy

@hsearcy hsearcy commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Problem

On Linux, Electron's safeStorage requires the --password-store Chromium switch to select the correct keychain backend. Without it, safeStorage.isEncryptionAvailable() returns false, apps/desktop/electron/hardening.ts refuses to persist remote gateway tokens, and Hermes Desktop silently falls back to requiring HERMES_DESKTOP_REMOTE_URL/HERMES_DESKTOP_REMOTE_TOKEN env vars for every remote gateway login.

Solution

hermes_cli/main.py — adds _detect_linux_password_store() which probes (in order):

  1. KDE_SESSION_VERSIONkwallet6 / kwallet5 / kwallet
  2. KDE_FULL_SESSIONkwallet
  3. GNOME_KEYRING_CONTROLgnome-libsecret (fast path, no subprocess)
  4. D-Bus ping of org.freedesktop.secretsgnome-libsecret (covers GNOME Keyring, KeePassXC secret service, etc.)

The result is bridged into the desktop subprocess env as HERMES_DESKTOP_PASSWORD_STORE on Linux, for both the source (npm exec electron .) and packaged launch paths.

User override via config.yaml (not a new user-facing HERMES_* env var, per AGENTS.md): desktop.password_store joins the existing desktop.* launch options in _desktop_launch_options(). "auto" (default) means detect; any of gnome-libsecret / kwallet / kwallet5 / kwallet6 / basic forces a backend. Precedence matches desktop.disable_gpu: explicit env var > config > detection.

apps/desktop/electron/bootstrap-platform.tsresolveLinuxPasswordStore() validates the bridged value against the known-good set; main.ts applies it via app.commandLine.appendSwitch('password-store', value) before app ready. Unrecognised values log a warning and are skipped; non-Linux platforms ignore the variable entirely.

Tests

  • tests/hermes_cli/test_gui_command.py: detector probes (KDE / GNOME / D-Bus / no-keychain), bridging on both packaged and source launch paths, config-override-skips-detection, env-var precedence, and linux-only gating.
  • apps/desktop/electron/bootstrap-platform.test.ts (vitest electron project): valid backends applied, unset/blank no-op, non-Linux no-op, unknown values warn instead of apply.

Impact

Platform / Setup Effect
macOS / Windows No change
Linux + GNOME Keyring Auto-detected, secure storage just works
Linux + KDE KWallet Auto-detected via KDE_SESSION_VERSION
Linux + other Secret Service impl Detected via D-Bus probe
Linux, no keychain daemon No change; existing error guidance unchanged
desktop.password_store set in config.yaml Forces that backend, detection skipped
HERMES_DESKTOP_PASSWORD_STORE already set Respected as-is; wins over config and detection

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have labels Jun 7, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing Linux secure token storage. The underlying gap is still present: apps/desktop/electron/hardening.ts:36-45 rejects token persistence when safeStorage.isEncryptionAvailable() is false, and current main has no password-store setup.

Problems

  • The Electron change targets apps/desktop/electron/main.cjs, which no longer exists on current main. Commit 39d09453f migrated it to apps/desktop/electron/main.ts; apps/desktop/scripts/bundle-electron-main.mjs:22-43 bundles that TypeScript file into the packaged entrypoint.
  • The diff has no tests for backend detection or launcher propagation. Existing seams are tests/hermes_cli/test_gui_command.py:80-109 and the Electron platform test runner in apps/desktop/package.json:41.
  • The proposed user override is a new non-secret HERMES_* behavior setting. AGENTS.md:102-107 requires user-facing behavior configuration to live in config.yaml; current desktop.electron_flags handling is at hermes_cli/main.py:5588-5623.

Suggested changes

  • Salvage the implementation into main.ts, route any user override through desktop config, and add detector/propagation tests for source and packaged launch paths.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
…orage

On Linux, Electron's safeStorage requires the --password-store Chromium
switch to select the correct keychain backend. Without it,
isEncryptionAvailable() returns false, hardening.ts refuses to persist
remote gateway tokens, and users are forced back to the
HERMES_DESKTOP_REMOTE_URL / HERMES_DESKTOP_REMOTE_TOKEN env fallback.

- hermes_cli/main.py: _detect_linux_password_store() probes KDE session
  env vars, GNOME Keyring's control socket, then a D-Bus ping of
  org.freedesktop.secrets (covers any Secret Service implementation,
  e.g. KeePassXC). The result is bridged into the desktop subprocess env
  as HERMES_DESKTOP_PASSWORD_STORE for both source and packaged launches.

- The user override lives in config.yaml (desktop.password_store,
  default "auto") rather than a new user-facing HERMES_* env var, per
  AGENTS.md. An explicit HERMES_DESKTOP_PASSWORD_STORE env var still
  wins over config and detection, matching desktop.disable_gpu
  semantics.

- apps/desktop/electron/bootstrap-platform.ts:
  resolveLinuxPasswordStore() validates the bridged value; main.ts
  applies it via app.commandLine.appendSwitch('password-store', ...)
  before app ready. Unknown values log a warning and are skipped.

- Tests: detector + bridging coverage (packaged and source launch
  paths, config override, env-var precedence, linux-only gating) in
  tests/hermes_cli/test_gui_command.py; resolver coverage in
  bootstrap-platform.test.ts (vitest electron project).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hsearcy
hsearcy force-pushed the linux-keychain-auto-detect branch from 7824dfd to 66a2a4c Compare July 14, 2026 14:24
@hsearcy

hsearcy commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Reworked per the sweeper review — the branch is rebuilt on current main:

  • main.cjsmain.ts: the Electron change now lives in bootstrap-platform.ts as a pure resolveLinuxPasswordStore() helper (same home as detectRemoteDisplay), applied in main.ts pre-ready and picked up by scripts/bundle-electron-main.mjs like the rest of the TS entrypoint.
  • User override moved to config.yaml: desktop.password_store (default "auto") joins the existing desktop.* options in _desktop_launch_options(). HERMES_DESKTOP_PASSWORD_STORE remains only as the internal bridge env var, with the same precedence semantics as desktop.disable_gpu (explicit env > config > detection).
  • Tests added: detector + bridging coverage in tests/hermes_cli/test_gui_command.py for both the source and packaged launch paths (79 pass), and resolver coverage in bootstrap-platform.test.ts under the vitest electron project (417 pass). tsc -p tsconfig.electron.json and the esbuild bundle are clean.

@GottZ GottZ 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.

This was generated by AI during triage.

Graph note (no action implied — a maintainer has already reviewed this thread).

Our triage graph places this PR in a complex with 1 related pull request (#65087). They were checked against each other at the diff level and no consolidation is indicated — they address distinct causes.

Full neighbourhood: https://hermes-triage.gottz.de/?node=41236

This note exists so the relationship stays discoverable from the thread itself.

Resolves conflicts from upstream's DEFAULT_CONFIG extraction into
hermes_cli/config_defaults.py (password_store default moved there) and
the test-pruning waves (dropped the pruned pre-existing launch-option
tests; kept the new password-store tests).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@OutThisLife OutThisLife closed this pull request by merging all changes into NousResearch:main in 0c3f60f Aug 13, 2026
uaixo pushed a commit to uaixo/HTAI-Personal-Assistant that referenced this pull request Aug 13, 2026
uaixo pushed a commit to uaixo/HTAI-Personal-Assistant that referenced this pull request Aug 13, 2026
…e_packaged_executable

Main's helper no longer takes a platform kwarg (real-host layout since the
sys.platform-fake removal); mark the five password-store tests linux_only/
macos_only per the don't-fake-the-host policy, and stub the Linux desktop-entry
registration those cmd_gui runs now reach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants