mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-19 15:18:03 +00:00
Merge pull request #44085 from kshitijk4poor/review/pr-43754-ssh-update
Some checks failed
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build and Publish / build-amd64 (push) Waiting to run
Docker Build and Publish / build-arm64 (push) Waiting to run
Docker Build and Publish / merge (push) Blocked by required conditions
Lint (ruff + ty) / ruff + ty diff (push) Waiting to run
Lint (ruff + ty) / ruff enforcement (blocking) (push) Waiting to run
Lint (ruff + ty) / Windows footguns (blocking) (push) Waiting to run
Nix Lockfile Fix / auto-fix-main (push) Waiting to run
Nix Lockfile Fix / fix (push) Waiting to run
Nix / nix (macos-latest) (push) Waiting to run
Nix / nix (ubuntu-latest) (push) Waiting to run
OSV-Scanner / Scan lockfiles (push) Waiting to run
Tests / test (1) (push) Waiting to run
Tests / test (2) (push) Waiting to run
Tests / test (3) (push) Waiting to run
Tests / test (4) (push) Waiting to run
Tests / test (5) (push) Waiting to run
Tests / test (6) (push) Waiting to run
Tests / save-durations (push) Blocked by required conditions
Tests / e2e (push) Waiting to run
Typecheck / typecheck (apps/bootstrap-installer) (push) Waiting to run
Typecheck / typecheck (apps/desktop) (push) Waiting to run
Typecheck / typecheck (apps/shared) (push) Waiting to run
Typecheck / typecheck (ui-tui) (push) Waiting to run
Typecheck / typecheck (web) (push) Waiting to run
uv.lock check / uv lock --check (push) Waiting to run
Docker / shell lint / Lint Dockerfile (hadolint) (push) Has been cancelled
Docker / shell lint / Lint docker/ shell scripts (shellcheck) (push) Has been cancelled
Some checks failed
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build and Publish / build-amd64 (push) Waiting to run
Docker Build and Publish / build-arm64 (push) Waiting to run
Docker Build and Publish / merge (push) Blocked by required conditions
Lint (ruff + ty) / ruff + ty diff (push) Waiting to run
Lint (ruff + ty) / ruff enforcement (blocking) (push) Waiting to run
Lint (ruff + ty) / Windows footguns (blocking) (push) Waiting to run
Nix Lockfile Fix / auto-fix-main (push) Waiting to run
Nix Lockfile Fix / fix (push) Waiting to run
Nix / nix (macos-latest) (push) Waiting to run
Nix / nix (ubuntu-latest) (push) Waiting to run
OSV-Scanner / Scan lockfiles (push) Waiting to run
Tests / test (1) (push) Waiting to run
Tests / test (2) (push) Waiting to run
Tests / test (3) (push) Waiting to run
Tests / test (4) (push) Waiting to run
Tests / test (5) (push) Waiting to run
Tests / test (6) (push) Waiting to run
Tests / save-durations (push) Blocked by required conditions
Tests / e2e (push) Waiting to run
Typecheck / typecheck (apps/bootstrap-installer) (push) Waiting to run
Typecheck / typecheck (apps/desktop) (push) Waiting to run
Typecheck / typecheck (apps/shared) (push) Waiting to run
Typecheck / typecheck (ui-tui) (push) Waiting to run
Typecheck / typecheck (web) (push) Waiting to run
uv.lock check / uv lock --check (push) Waiting to run
Docker / shell lint / Lint Dockerfile (hadolint) (push) Has been cancelled
Docker / shell lint / Lint docker/ shell scripts (shellcheck) (push) Has been cancelled
fix(update): avoid SSH auth for passive official checks
This commit is contained in:
commit
955fa40062
6 changed files with 267 additions and 3 deletions
|
|
@ -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,
|
||||
|
|
@ -1312,6 +1316,11 @@ function runGit(args, options = {}) {
|
|||
|
||||
const firstLine = text => (text || '').split('\n').find(Boolean) || ''
|
||||
|
||||
async function getOriginUrl(updateRoot) {
|
||||
const origin = await runGit(['remote', 'get-url', 'origin'], { cwd: updateRoot })
|
||||
return origin.code === 0 ? origin.stdout.trim() : ''
|
||||
}
|
||||
|
||||
function emitUpdateProgress(payload) {
|
||||
const merged = { stage: 'idle', message: '', percent: null, error: null, ...payload, at: Date.now() }
|
||||
rememberLog(`[updates] ${merged.stage}: ${merged.message || merged.error || ''}`)
|
||||
|
|
@ -1331,7 +1340,9 @@ async function resolveHealedBranch(updateRoot, branch) {
|
|||
return branch || 'main'
|
||||
}
|
||||
|
||||
const probe = await runGit(['ls-remote', '--exit-code', '--heads', 'origin', branch], { cwd: updateRoot })
|
||||
const originUrl = await getOriginUrl(updateRoot)
|
||||
const remote = isOfficialSshRemote(originUrl) ? OFFICIAL_REPO_HTTPS_URL : 'origin'
|
||||
const probe = await runGit(['ls-remote', '--exit-code', '--heads', remote, branch], { cwd: updateRoot })
|
||||
if (probe.code !== 2) {
|
||||
return branch
|
||||
}
|
||||
|
|
@ -1359,6 +1370,40 @@ async function checkUpdates() {
|
|||
}
|
||||
|
||||
branch = await resolveHealedBranch(updateRoot, branch)
|
||||
const originUrl = await getOriginUrl(updateRoot)
|
||||
if (isOfficialSshRemote(originUrl)) {
|
||||
const git = args => runGit(args, { cwd: updateRoot }).then(r => r.stdout.trim())
|
||||
const [currentSha, target, dirtyStr, currentBranch] = await Promise.all([
|
||||
git(['rev-parse', 'HEAD']),
|
||||
runGit(['ls-remote', OFFICIAL_REPO_HTTPS_URL, `refs/heads/${branch}`], { cwd: updateRoot }),
|
||||
git(['status', '--porcelain']),
|
||||
git(['rev-parse', '--abbrev-ref', 'HEAD'])
|
||||
])
|
||||
const targetSha = firstLine(target.stdout).split(/\s+/)[0] || ''
|
||||
if (target.code !== 0 || !targetSha) {
|
||||
return {
|
||||
supported: true,
|
||||
branch,
|
||||
error: 'fetch-failed',
|
||||
message: firstLine(target.stderr) || 'git ls-remote failed.',
|
||||
hermesRoot: updateRoot,
|
||||
fetchedAt: Date.now()
|
||||
}
|
||||
}
|
||||
return {
|
||||
supported: true,
|
||||
branch,
|
||||
currentBranch,
|
||||
behind: currentSha && currentSha === targetSha ? 0 : 1,
|
||||
currentSha,
|
||||
targetSha,
|
||||
commits: [],
|
||||
dirty: dirtyStr.length > 0,
|
||||
hermesRoot: updateRoot,
|
||||
fetchedAt: Date.now()
|
||||
}
|
||||
}
|
||||
|
||||
const fetched = await runGit(['fetch', '--quiet', 'origin', branch], { cwd: updateRoot })
|
||||
if (fetched.code !== 0) {
|
||||
return {
|
||||
|
|
|
|||
56
apps/desktop/electron/update-remote.cjs
Normal file
56
apps/desktop/electron/update-remote.cjs
Normal file
|
|
@ -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
|
||||
}
|
||||
78
apps/desktop/electron/update-remote.test.cjs
Normal file
78
apps/desktop/electron/update-remote.test.cjs
Normal file
|
|
@ -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)
|
||||
})
|
||||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import subprocess
|
|||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlparse
|
||||
from hermes_constants import get_hermes_home
|
||||
from typing import TYPE_CHECKING, Dict, List, Optional
|
||||
|
||||
|
|
@ -121,6 +122,53 @@ _UPDATE_CHECK_CACHE_SECONDS = 6 * 3600
|
|||
UPDATE_AVAILABLE_NO_COUNT = -1
|
||||
|
||||
_UPSTREAM_REPO_URL = "https://github.com/NousResearch/hermes-agent.git"
|
||||
_OFFICIAL_REPO_CANONICAL = "github.com/nousresearch/hermes-agent"
|
||||
|
||||
|
||||
def _canonical_github_remote(url: str | None) -> str:
|
||||
"""Return ``host/owner/repo`` for common GitHub remote URL forms."""
|
||||
if not url:
|
||||
return ""
|
||||
value = url.strip()
|
||||
if value.startswith("git@github.com:"):
|
||||
value = "github.com/" + value[len("git@github.com:"):]
|
||||
elif value.startswith("ssh://git@github.com/"):
|
||||
value = "github.com/" + value[len("ssh://git@github.com/"):]
|
||||
else:
|
||||
parsed = urlparse(value)
|
||||
if parsed.netloc and parsed.path:
|
||||
value = f"{parsed.netloc}{parsed.path}"
|
||||
value = value.strip().rstrip("/")
|
||||
if value.endswith(".git"):
|
||||
value = value[:-4]
|
||||
return value.lower()
|
||||
|
||||
|
||||
def _is_ssh_remote(url: str | None) -> bool:
|
||||
if not url:
|
||||
return False
|
||||
value = url.strip().lower()
|
||||
return value.startswith("git@") or value.startswith("ssh://")
|
||||
|
||||
|
||||
def _is_official_ssh_remote(url: str | None) -> bool:
|
||||
return _is_ssh_remote(url) and _canonical_github_remote(url) == _OFFICIAL_REPO_CANONICAL
|
||||
|
||||
|
||||
def _git_stdout(args: list[str], *, cwd: Path, timeout: int = 5) -> Optional[str]:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["git", *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=timeout,
|
||||
cwd=str(cwd),
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
if result.returncode != 0:
|
||||
return None
|
||||
return (result.stdout or "").strip()
|
||||
|
||||
|
||||
def _check_via_rev(local_rev: str) -> Optional[int]:
|
||||
|
|
@ -146,6 +194,11 @@ def _check_via_rev(local_rev: str) -> Optional[int]:
|
|||
|
||||
def _check_via_local_git(repo_dir: Path) -> Optional[int]:
|
||||
"""Count commits behind origin/main in a local checkout."""
|
||||
origin_url = _git_stdout(["remote", "get-url", "origin"], cwd=repo_dir)
|
||||
if _is_official_ssh_remote(origin_url):
|
||||
head_rev = _git_stdout(["rev-parse", "HEAD"], cwd=repo_dir)
|
||||
return _check_via_rev(head_rev) if head_rev else None
|
||||
|
||||
try:
|
||||
subprocess.run(
|
||||
["git", "fetch", "origin", "--quiet"],
|
||||
|
|
|
|||
|
|
@ -93,7 +93,39 @@ def test_check_for_updates_expired_cache(tmp_path, monkeypatch):
|
|||
result = check_for_updates()
|
||||
|
||||
assert result == 5
|
||||
assert mock_run.call_count == 2 # git fetch + git rev-list
|
||||
assert mock_run.call_count == 3 # origin probe + git fetch + git rev-list
|
||||
|
||||
|
||||
def test_check_for_updates_official_ssh_origin_uses_https_probe(tmp_path):
|
||||
"""Passive update checks must not trigger SSH auth for official installs."""
|
||||
import hermes_cli.banner as banner
|
||||
|
||||
repo_dir = tmp_path / "hermes-agent"
|
||||
repo_dir.mkdir()
|
||||
(repo_dir / ".git").mkdir()
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
calls.append(cmd)
|
||||
if cmd == ["git", "remote", "get-url", "origin"]:
|
||||
return MagicMock(returncode=0, stdout="git@github.com:NousResearch/hermes-agent.git\n")
|
||||
if cmd == ["git", "rev-parse", "HEAD"]:
|
||||
return MagicMock(returncode=0, stdout="local-sha\n")
|
||||
if cmd == [
|
||||
"git",
|
||||
"ls-remote",
|
||||
"https://github.com/NousResearch/hermes-agent.git",
|
||||
"refs/heads/main",
|
||||
]:
|
||||
return MagicMock(returncode=0, stdout="upstream-sha\trefs/heads/main\n")
|
||||
raise AssertionError(f"unexpected git command: {cmd!r}")
|
||||
|
||||
with patch("hermes_cli.banner.subprocess.run", side_effect=fake_run):
|
||||
result = banner._check_via_local_git(repo_dir)
|
||||
|
||||
assert result == banner.UPDATE_AVAILABLE_NO_COUNT
|
||||
assert ["git", "fetch", "origin", "--quiet"] not in calls
|
||||
|
||||
|
||||
def test_check_for_updates_no_git_dir(tmp_path, monkeypatch):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue