From 0edeee14c6ce02a58df436638d3ac47dbb4bcd2d Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:53:19 +0530 Subject: [PATCH] test(desktop): cover official-SSH remote detection for passive updates Extract the remote-detection helpers (canonicalGitHubRemote, isSshRemote, isOfficialSshRemote) from main.cjs into a testable update-remote.cjs sibling module and add a node:test suite, wired into test:desktop:platforms. main.cjs requires('electron') at load, so its inline helpers weren't unit testable. The Python side of #43754 shipped a regression test; this gives the desktop side the same coverage for the security-critical detection that keeps passive update checks off the SSH origin (avoiding FIDO2/passkey touch prompts). Tests assert SSH/HTTPS forms canonicalize equal, official SSH is detected case-insensitively, and forks / other hosts / the HTTPS remote are NOT misclassified. --- apps/desktop/electron/main.cjs | 35 +-------- apps/desktop/electron/update-remote.cjs | 56 ++++++++++++++ apps/desktop/electron/update-remote.test.cjs | 78 ++++++++++++++++++++ apps/desktop/package.json | 2 +- 4 files changed, 139 insertions(+), 32 deletions(-) create mode 100644 apps/desktop/electron/update-remote.cjs create mode 100644 apps/desktop/electron/update-remote.test.cjs diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index dcd30ed5432..5af4b5605ce 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -31,6 +31,10 @@ const { canImportHermesCli, verifyHermesCli } = require('./backend-probes.cjs') const { probeGatewayWebSocket } = require('./gateway-ws-probe.cjs') const { serializeJsonBody, setJsonRequestHeaders } = require('./oauth-net-request.cjs') const { fetchMarketplaceThemes, searchMarketplaceThemes } = require('./vscode-marketplace.cjs') +const { + OFFICIAL_REPO_HTTPS_URL, + isOfficialSshRemote +} = require('./update-remote.cjs') const { buildPosixCleanupScript, buildWindowsCleanupScript, @@ -278,8 +282,6 @@ const PROFILE_NAME_RE = /^[a-z0-9][a-z0-9_-]{0,63}$/ // tracks main. User can also override at runtime via // hermesDesktop.updates.setBranch(). const DEFAULT_UPDATE_BRANCH = 'main' -const OFFICIAL_REPO_HTTPS_URL = 'https://github.com/NousResearch/hermes-agent.git' -const OFFICIAL_REPO_CANONICAL = 'github.com/nousresearch/hermes-agent' // desktop.log lives under HERMES_HOME/logs/ so it sits next to agent.log, // errors.log, gateway.log produced by hermes_logging.setup_logging — one log // directory per user, regardless of which UI surface produced the line. @@ -1314,35 +1316,6 @@ function runGit(args, options = {}) { const firstLine = text => (text || '').split('\n').find(Boolean) || '' -function canonicalGitHubRemote(url) { - if (!url) return '' - let value = String(url).trim() - if (value.startsWith('git@github.com:')) { - value = `github.com/${value.slice('git@github.com:'.length)}` - } else if (value.startsWith('ssh://git@github.com/')) { - value = `github.com/${value.slice('ssh://git@github.com/'.length)}` - } else { - try { - const parsed = new URL(value) - if (parsed.hostname && parsed.pathname) value = `${parsed.hostname}${parsed.pathname}` - } catch { - // Leave non-URL forms unchanged. - } - } - value = value.trim().replace(/\/+$/, '') - if (value.endsWith('.git')) value = value.slice(0, -4) - return value.toLowerCase() -} - -function isSshRemote(url) { - const value = String(url || '').trim().toLowerCase() - return value.startsWith('git@') || value.startsWith('ssh://') -} - -function isOfficialSshRemote(url) { - return isSshRemote(url) && canonicalGitHubRemote(url) === OFFICIAL_REPO_CANONICAL -} - async function getOriginUrl(updateRoot) { const origin = await runGit(['remote', 'get-url', 'origin'], { cwd: updateRoot }) return origin.code === 0 ? origin.stdout.trim() : '' diff --git a/apps/desktop/electron/update-remote.cjs b/apps/desktop/electron/update-remote.cjs new file mode 100644 index 00000000000..3cb432d1b1e --- /dev/null +++ b/apps/desktop/electron/update-remote.cjs @@ -0,0 +1,56 @@ +/** + * Pure helpers for choosing a remote URL during passive update checks. + * + * A public install can end up with `origin=git@github.com:NousResearch/hermes-agent.git`. + * If the user's GitHub SSH key is FIDO2/passkey-backed, a background `git fetch + * origin` triggers an unexplained hardware-touch prompt. For passive checks + * against the official repo we substitute the public HTTPS `ls-remote` path, + * which needs no auth and cannot prompt. Active update/apply flows are left + * unchanged. + * + * Extracted from main.cjs so the security-critical remote detection is unit + * testable without booting Electron (main.cjs requires('electron') at load). + */ + +const OFFICIAL_REPO_HTTPS_URL = 'https://github.com/NousResearch/hermes-agent.git' +const OFFICIAL_REPO_CANONICAL = 'github.com/nousresearch/hermes-agent' + +// Normalize common GitHub remote URL forms to `host/owner/repo` (lowercased, +// no trailing slash, no .git suffix) so SSH and HTTPS forms of the same repo +// compare equal. +function canonicalGitHubRemote(url) { + if (!url) return '' + let value = String(url).trim() + if (value.startsWith('git@github.com:')) { + value = `github.com/${value.slice('git@github.com:'.length)}` + } else if (value.startsWith('ssh://git@github.com/')) { + value = `github.com/${value.slice('ssh://git@github.com/'.length)}` + } else { + try { + const parsed = new URL(value) + if (parsed.hostname && parsed.pathname) value = `${parsed.hostname}${parsed.pathname}` + } catch { + // Leave non-URL forms unchanged. + } + } + value = value.trim().replace(/\/+$/, '') + if (value.endsWith('.git')) value = value.slice(0, -4) + return value.toLowerCase() +} + +function isSshRemote(url) { + const value = String(url || '').trim().toLowerCase() + return value.startsWith('git@') || value.startsWith('ssh://') +} + +function isOfficialSshRemote(url) { + return isSshRemote(url) && canonicalGitHubRemote(url) === OFFICIAL_REPO_CANONICAL +} + +module.exports = { + OFFICIAL_REPO_HTTPS_URL, + OFFICIAL_REPO_CANONICAL, + canonicalGitHubRemote, + isSshRemote, + isOfficialSshRemote +} diff --git a/apps/desktop/electron/update-remote.test.cjs b/apps/desktop/electron/update-remote.test.cjs new file mode 100644 index 00000000000..0dfba970138 --- /dev/null +++ b/apps/desktop/electron/update-remote.test.cjs @@ -0,0 +1,78 @@ +/** + * Tests for electron/update-remote.cjs — the remote-detection helpers that + * keep passive update checks off the SSH origin for official installs. + * + * Run with: node --test electron/update-remote.test.cjs + * (Wired into npm test:desktop:platforms in package.json.) + * + * Why this matters: a public install can carry + * origin=git@github.com:NousResearch/hermes-agent.git. A background + * `git fetch origin` then authenticates over SSH and, with a FIDO2/passkey + * key, triggers an unexplained hardware-touch prompt. isOfficialSshRemote + * must reliably recognize the official SSH remote (in every URL form, + * case-insensitively) so the caller can swap in the anonymous HTTPS path — + * while NOT misclassifying forks, other hosts, or the HTTPS remote (which + * never prompts and should keep the normal fetch path). + */ + +const test = require('node:test') +const assert = require('node:assert/strict') + +const { + OFFICIAL_REPO_HTTPS_URL, + OFFICIAL_REPO_CANONICAL, + canonicalGitHubRemote, + isSshRemote, + isOfficialSshRemote +} = require('./update-remote.cjs') + +test('canonicalGitHubRemote normalizes SSH and HTTPS forms to the same value', () => { + assert.equal(canonicalGitHubRemote('git@github.com:NousResearch/hermes-agent.git'), OFFICIAL_REPO_CANONICAL) + assert.equal(canonicalGitHubRemote('git@github.com:NousResearch/hermes-agent'), OFFICIAL_REPO_CANONICAL) + assert.equal(canonicalGitHubRemote('ssh://git@github.com/NousResearch/hermes-agent.git'), OFFICIAL_REPO_CANONICAL) + assert.equal(canonicalGitHubRemote('https://github.com/NousResearch/hermes-agent.git'), OFFICIAL_REPO_CANONICAL) + // Case-insensitive: an uppercased owner still canonicalizes to the same repo. + assert.equal(canonicalGitHubRemote('git@github.com:nousresearch/hermes-agent.git'), OFFICIAL_REPO_CANONICAL) + // Trailing slashes are stripped. + assert.equal(canonicalGitHubRemote('https://github.com/NousResearch/hermes-agent/'), OFFICIAL_REPO_CANONICAL) +}) + +test('canonicalGitHubRemote is empty for falsy input', () => { + assert.equal(canonicalGitHubRemote(''), '') + assert.equal(canonicalGitHubRemote(null), '') + assert.equal(canonicalGitHubRemote(undefined), '') +}) + +test('isSshRemote detects scp-like and ssh:// forms only', () => { + assert.equal(isSshRemote('git@github.com:NousResearch/hermes-agent.git'), true) + assert.equal(isSshRemote('ssh://git@github.com/NousResearch/hermes-agent.git'), true) + assert.equal(isSshRemote('https://github.com/NousResearch/hermes-agent.git'), false) + assert.equal(isSshRemote(''), false) + assert.equal(isSshRemote(null), false) +}) + +test('isOfficialSshRemote is true only for the official repo over SSH', () => { + assert.equal(isOfficialSshRemote('git@github.com:NousResearch/hermes-agent.git'), true) + assert.equal(isOfficialSshRemote('git@github.com:NousResearch/hermes-agent'), true) + assert.equal(isOfficialSshRemote('ssh://git@github.com/NousResearch/hermes-agent.git'), true) + // Case-insensitive owner/repo match. + assert.equal(isOfficialSshRemote('git@github.com:nousresearch/hermes-agent.git'), true) +}) + +test('isOfficialSshRemote does NOT match forks, other hosts, or HTTPS', () => { + // A fork over SSH belongs to the user — fetching it is their own remote, + // not the official upstream, so the SSH-avoidance swap must not apply. + assert.equal(isOfficialSshRemote('git@github.com:someuser/hermes-agent.git'), false) + // Same repo name on a different host is not the official repo. + assert.equal(isOfficialSshRemote('git@gitlab.com:NousResearch/hermes-agent.git'), false) + // HTTPS to the official repo never prompts for SSH/FIDO2, so it keeps the + // normal fetch path — must not be flagged as an official SSH remote. + assert.equal(isOfficialSshRemote('https://github.com/NousResearch/hermes-agent.git'), false) + assert.equal(isOfficialSshRemote(''), false) + assert.equal(isOfficialSshRemote(null), false) +}) + +test('OFFICIAL_REPO_HTTPS_URL canonicalizes to OFFICIAL_REPO_CANONICAL', () => { + // Invariant: the URL we substitute in must be the same repo we detect. + assert.equal(canonicalGitHubRemote(OFFICIAL_REPO_HTTPS_URL), OFFICIAL_REPO_CANONICAL) +}) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index e373fc78825..e45ec8e804b 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -35,7 +35,7 @@ "test:desktop:nsis": "node scripts/test-desktop.mjs nsis", "test:desktop:existing": "node scripts/test-desktop.mjs existing", "test:desktop:fresh": "node scripts/test-desktop.mjs fresh", - "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/windows-child-process.test.cjs", + "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs", "typecheck": "tsc -p . --noEmit", "lint": "eslint src/ electron/", "lint:fix": "eslint src/ electron/ --fix",