Review feedback on #88136 (monerostar): a profile-scoped `hermes update`
sets HERMES_HOME to <root>/profiles/<name>, but the Hermes-managed
PortableGit tree lives under the SHARED root (<root>/git/...). The locator
checked get_hermes_home() only, so a broken trampoline during a
profile-scoped update was not swapped and fell through to ZIP.
Extract _portable_git_candidates() (shared root first, profile home as
fallback) and add a regression test for the profile layout.
A Git-for-Windows trampoline launcher (bin\git.exe / cmd\git.exe shim,
~46KB) that fails to re-exec the real git-core binary refuses every git
call with a "BUG (fork bomb)" guard instead of running it (#87876).
Detect the trampoline up front via `git --version`, locate a real git
binary (Git for Windows or Hermes-managed PortableGit locations), and
rebuild the git command with it so fetch/pull/checkout keep working with
a real git instead of degrading to the ZIP fallback. When no real binary
can be found, leave the command untouched so the existing fetch-failure
handler still falls back to the ZIP path on Windows (#88046).
Builds on webtecnica's escape-aware _split_key_path (#84152, cherry-picked
with authorship preserved; earliest fix in the family was RelaxJonh's #80253
greedy-match approach — both behaviors now ship together):
- _greedy_literal_match: when navigating an EXISTING mapping, prefer an
existing literal key equal to the dot-join of the next N path segments
(longest match wins). Dotted model IDs are the norm, so the common
unescaped command (config set providers.p.models.grok-4.6.supports_vision
true) now hits the real key across set/get/unset instead of creating a
phantom sibling. Plain dotted paths with no dotted-key collision split
exactly as before.
- _phantom_sibling + ValueError in _set_nested: refuse to CREATE a new
intermediate mapping that would shadow an existing dotted literal sibling
(Soju06's fail-loudly suggestion on #84064); set_config_value surfaces it
as a clean CLI error with the escaped spelling to use.
- utils.py::atomic_roundtrip_yaml_update (the second split site, #91607 —
/model + TUI persistence) now uses the same escape-aware split + greedy
literal matching.
- CFG-04 empty-segment guard now splits escape-aware so escaped keys are
not misclassified.
- Tests for every repro shape in the family: #84064 provider model keys,
#80006 Matrix room IDs, #91095 dotted models under custom_providers list
index (incl. escaped creation-when-absent), #91607 model_overrides via
atomic_roundtrip_yaml_update, #99124 dotted leaf keys; plus
backward-compat coverage. Also fixed the carrier's one stale assertion
(structured-value coercion landed on main after #84152 branched) and
removed its dead _MCP_SECRETS_CONFIG fixture flagged in review.
- Docs: 'Dots inside key names' section in website/docs/reference/cli-commands.md.
Fixes#84064, fixes#80006, fixes#91095, fixes#91607, fixes#99124
Generalize the HERMES_S6_SUPERVISED_CHILD supervisor-marker mechanism so
ANY supervised gateway launch (systemd, launchd, Windows Scheduled Task,
external supervisor) skips the active_profile redirect in
_apply_profile_override(). Previously only the s6 container marker was
honored, so a systemd-launched default-profile gateway with
HERMES_HOME=<root> followed the sticky active_profile file and silently
assumed another profile's identity — logging under that profile's tree
and connecting with its Telegram bot token (double-polling a token owned
by that profile's own live gateway).
- hermes_cli/main.py: honor HERMES_SUPERVISED_CHILD (new generalized
marker), HERMES_S6_SUPERVISED_CHILD (back-compat), INVOCATION_ID
(systemd; gateway commands only, since it leaks into every descendant
of systemd-launched processes), and HERMES_GATEWAY_EXTERNAL_SUPERVISOR.
- hermes_cli/gateway.py: export HERMES_SUPERVISED_CHILD=1 in generated
systemd units (user + system) and the launchd plist.
- hermes_cli/gateway_windows.py: export it from the Scheduled-Task cmd/vbs
launchers and the windowless respawn env overlay.
- hermes_cli/service_manager.py: export it alongside the s6 sentinel.
- tests: regression coverage for all markers + non-gateway INVOCATION_ID
neutrality + generated-unit marker presence.
Fixes#74872
tarfile.open(archive_path, "w:gz") truncates the destination the instant
it opens. If tf.add() fails partway (disk full, permission loss,
interruption), whatever was previously at that path is gone — including
an existing profile or board export the caller chose to overwrite. This
is the same failure shape a7e7de6407 just fixed for the desktop gateway
file-save path, one commit earlier in the same window, but it was never
propagated to this shared archive-writing primitive even though board
export gained a new caller into it in that same window.
make_targz now writes into a sibling temp file (mkstemp, same directory
as the destination so the final step is a same-volume rename) and only
replaces the destination via os.replace() after the archive is fully
written and closed, mirroring the mkstemp+os.replace pattern already
used throughout this codebase (agent/secret_sources/_cache.py,
cron/jobs.py, gateway/status.py, etc). The temp file is unlinked on any
failure.
Salvaged from PR #84199 by @RickyYii. DirectAlias gains api_key/key_env; the direct-alias override re-resolves credentials against the alias endpoint (host-gated, #28660) and reuses the pre-alias key only on an origin match; oneshot -m <alias> passes the alias key as explicit_api_key; direct-alias branch gains the OLLAMA_API_KEY host gate. Fixes#83612.
Salvage hardening on top of the three cherry-picked contributor commits
(#91297 gebilaowang404 + AlexMnrs, #96741 burak33bb, #98826 ayushnangia),
closing the remaining unverified-PID kill sites as one class (#98814, #89614):
- pid_is_hermes: token-boundary 'hermes' match (no more loose substring
false-positives), and an explicit start-time expectation is now honored
on POSIX too (a mismatched fingerprint is a recycled PID on any platform).
- kill_process_tree: drop the guard on our OWN retained Popen child — a
retained handle pins the PID, so the check could only false-refuse.
- gateway.status.terminate_pid: POSIX force-kills also refuse when a
caller-provided expected_start_time no longer matches.
- kill_gateway_processes: re-verify the LIVE cmdline at kill time (the
scan-time match is a TOCTOU window).
- _reap_unsupervised_gateway_orphans: fingerprint orphans at scan time and
require a still-matching identity before the delayed SIGKILL escalation.
- whatsapp _kill_port_process: never kill a bare netstat/lsof-scanned PID
unless the live process is actually a node bridge (was a stranger-kill).
- browser daemon reap/close paths: pass the start-time fingerprint into
ProcessRegistry._terminate_host_pid (previously unverified), and the
session-close path now runs the same daemon identity verification as
the orphan reaper.
- tests/hermes_cli/test_taskkill_identity_windows_live.py: live Windows
probes (real spawned processes, real psutil ancestry) wired into the
on-demand windows-latest wine2e lane.
Fixes#98814Fixes#89614
Guard every Windows `taskkill /PID` against stale/recycled PIDs
(#89614: 8x 0xEF blue screens; a rebooted PID can be svchost.exe).
Adopted the community patch by AlexMnrs (commit 0162465): shared
psutil-based (pid, create_time) guard reusing the repo's existing
get_process_start_time machinery:
- fail closed on invalid/unknown/recycled identities (0/-1/None/bool/non-int)
- capture identity at discovery, re-validate at kill time
- all three sites through pid_is_hermes; taskkill stays hidden
Sites: _subprocess_compat.kill_process_tree,
dashboard_procs._kill_stale_dashboard_processes (win32),
update_cmd._stop_process_trees.
Refs #90471, #89614
Co-authored-by: Alex Monrás <AlexMnrs@users.noreply.github.com>
validate_platform_toolsets() accumulated a single valid_count across every
platform, so the "zero valid toolsets" safety net was suppressed as soon as any
one platform carried a valid toolset. A platform wiped to [] — the active one,
typically cli — therefore produced no warning at all.
resolve_enabled_toolsets() honours that empty list verbatim ([] is a list, so
the platform-default fallback is skipped), leaving the agent with zero tool
schemas. The model then has nothing to call and emits the tool call as
assistant text with finish_reason=stop: no error, no warning, no log entry.
That is the silent-failure mode this module was written to prevent (#38798).
Note the asymmetry this leaves intact: a malformed *string* value is not a list,
so it falls back to the platform default and fails open (#78103); an empty list
fails closed. The fail-closed resolution is deliberate (the explicit_empty_
selection contract in tools_config.py, and #82010 wants it persistable), so this
only adds the missing warning and does not change resolution semantics.
Fixes#89050
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rewrites resolve_exec_command so the generated .desktop Exec no longer
depends on how the installer happened to be launched: fixes the bare
repo-script form whose shebang escapes the venv, and the symlinked-venv
form that .resolve() dereferenced into the base interpreter store.
Salvaged squashed from PR #94874 (24 commits) after the original branch
was found to carry stray __pycache__/.gitignore payload.
Co-authored-by: Gökhan <gkhn.yldrmlr@gmail.com>
A non-loopback dashboard.public_url engaged the ticket-only auth gate for
EVERY hermes serve on the machine — including the private loopback
backends the Desktop app spawns for itself (HERMES_DESKTOP=1). Those
backends authenticate with the per-spawn session token, which the gated
WS path refuses outright, so Desktop failed to boot with:
Local Hermes backend is HTTP-reachable but the WebSocket (/api/ws)
rejected the session token.
The public_url describes a DIFFERENT deployment: the actual public
dashboard is a separate process on a non-loopback bind whose own startup
keeps its gate. Exempting Desktop-owned loopback backends therefore never
opens the public surface.
Exemption requires ALL of: loopback bind, HERMES_DESKTOP=1 (set by every
Desktop spawn path, local and SSH), and an operator-minted credential
(HERMES_DASHBOARD_SESSION_TOKEN, SSH session token, or owner nonce).
Non-Desktop serves and non-loopback binds keep the exact previous
behaviour — verified by regression tests on both sides of the boundary.
Fixes#96490
_denormalize_config_from_web only wrote model_context_length into the
on-disk model dict inside the branch gated on `model` also being present
in the payload. That was harmless when the frontend always sent the full
config, but the prior commit switched Settings autosave to send only the
diff (diffConfig), so editing the Context Window control alone omits
`model` from the payload and the context-length edit is silently thrown
away. The mirror case regressed too: editing `model` alone now omits
model_context_length from the diff, and the old code treated that missing
key the same as an explicit 0, wiping an existing context_length override
that the user never touched.
Track whether model_context_length was actually present in the payload
and only mutate context_length when it was, independent of whether
`model` also changed.
A venv ever touched by sudo pip / sudo hermes contains root-owned files
(classically site-packages/*.dist-info/INSTALLER). A later normal-user
'hermes update' pulls code fine, then 'uv pip install -e .' dies with
'Permission denied (os error 13)' mid-mutation — venv/bin/hermes already
deleted, CLI bricked.
Add a bounded, pure-stat ownership preflight (_venv_foreign_owned_paths)
that runs after the code pull and immediately before the dependency
install. If foreign-owned paths are found it refuses up front, names the
offending paths + owner uid, prints the exact recovery command
(sudo chown -R $(id -un): <root>), and confirms the venv is untouched.
Windows (no os.geteuid) and root skip entirely. Never raises, capped at
~2000 stat calls, no subprocess use (update tests mock subprocess.run).
Same refuse-before-mutate philosophy as the contended-venv gate (#87331).
Fixes#83529
Diagnosis and documented recovery by @eabase.
Widens the salvaged cron doctor with the highest-value fleet check:
an active job whose next_run_at is parked >15min in the past is not
firing (dead ticker, downed gateway, wedged fire-claim). Also registers
doctor in the docs (cron guide + CLI reference) and resolves the salvage
onto current main alongside runs/incidents/notepad.
Companion to #98935, which fixes _fts_table_probe itself. This covers the
surfaces that PR does not touch:
- web_server._open_session_db_at_path: the one-writable-open heal only
caught sqlite3.DatabaseError; a raw UnicodeDecodeError (pysqlite failing
to decode SQLite's own error message over corrupt file bytes) bypassed
it, so the heal documented for malformed schema never fired (#98924
Failure 1). Both catches widened; decode errors dispatch to the heal.
- SessionSchemaMixin._recover_stale_fts_locked: drop-and-recreate skipped
vtables whose probe raised UnicodeDecodeError, the same too-narrow
catch the issue identified in the probe.
- TUI gateway: _ensure_session_db_row returned silently when the store
could not open, so prompt.submit streamed the turn while persisting
nothing (#98924 Failure 2). It now returns False and prompt.submit
fails the RPC with code 5072 so desktop maps it to a toast, mirroring
the disk-full/5070 convention. session.create stays silent per its
pinned degraded-mode contract.
Browser Use mode never read browser.engine: _resolve_backend_cdp() went
BU_CDP_* env -> CDP override -> cloud provider -> local Chrome, so
`engine: lightpanda` was a silent no-op on the default backend, and on
the built-in path it was skipped whenever a cloud provider, Camofox or a
CDP override was active without anyone saying so.
- browser_use_cli: when the engine is lightpanda and nothing with higher
precedence claimed the session, get a session from _get_session_info()
and export its endpoint as BU_CDP_URL; the browser is private to the
session key, so the own-tab preamble is skipped. The browser_exec
description gains a Lightpanda header (text-first, new_tab once then
goto_url — lightpanda-io/browser#1962).
- browser_tool: _create_local_session() spawns `lightpanda serve
--host 127.0.0.1 --port <free>` per session key (new
tools/browser_lightpanda.py), reusing the session cache, inactivity
reaper and atexit cleanup; a dead process is respawned on the next call;
orphans from a crashed Hermes are reaped through per-process records in
$HERMES_HOME/cache/browser-use/lightpanda/. New lightpanda_engine_status()
reports whether the engine is in effect or what shadows it.
- tools_config: "Lightpanda" row in the Browser Automation picker
(cloud_provider: local + engine: lightpanda; "Local Browser" resets the
engine to auto) with a binary-check post-setup.
- /browser status and hermes doctor print the engine state and, when it
is shadowed, the reason.
Follow-up to the salvaged manifest guard (#90859): the doctor's temporary
HERMES_HOME now enters the ExitStack before the staging copytree, so
ENOSPC / KeyboardInterrupt / any exception during staging deterministically
removes the hermes-plugin-doctor-* directory instead of relying on the
TemporaryDirectory GC finalizer (which cannot run while the traceback pins
the frame). Regression test proven via sabotage run against the old code.
`hermes plugins doctor` defaults its target to `.`, and
resolve_plugin_path accepted any directory that existed. Doctor then
copytree'd the resolved path into a temporary HERMES_HOME *before* the
runtime got to reject it, so running the command from a directory that
is not a plugin copied that whole tree.
Run from $HOME on macOS this copies the home directory, and because
`Library/CloudStorage` is not excluded it also materializes every
cloud-only Google Drive/iCloud placeholder. Observed locally: 461 GB
written to /private/var/folders and still growing when the process was
killed, on a machine with 49 GB free.
Resolve now requires a manifest before returning a path, mirroring
PluginManager._scan_directory: `plugin.yaml`/`plugin.yml`/`plugin.json`
in the directory itself, or in one immediate subdirectory for the
category layout. Plugin-id candidates are only tried for values that can
be an id, since joining `.` onto a plugins root resolves to the root and
would hand Doctor every installed plugin at once.
Non-plugin targets now fail with a clear message and no copy.
/simplify-code efficiency reviewer (verified): cleanup_stale=False does
NOT deliver the exclusion the salvaged fix intended — get_running_pid
returns None whenever a record fails liveness VALIDATION (start-time
mismatch, argv drift, lock hiccup) regardless of the flag, which only
controls unlinking. In exactly the at-risk scenario the recorded PID
still never joined the exclusion set.
Exclusion evidence now comes from the RAW pidfile + lock records (no
validation, no unlink side effects); the validated non-destructive
probe is kept for the runtime-status fallback PID. For a KILL exclusion
list this is strictly safer: a stale recorded PID at worst spares one
process for one sweep, while a validation false-negative would
TerminateProcess a live gateway. Regression test reworked to drive the
real function semantics (raw record present, validation rejects);
mutation-checked: removing the raw-record read fails the test.
Follow-up on the salvaged #87158: the reaper-side fix (probe with
cleanup_stale=False so the sweep never deletes its own exclusion
evidence) fully protects a healthy standalone gateway, so the
web_server.py skip-sweep gate is dropped — a stale-but-present
registration must not veto the #77276 orphan reap that motivated the
sweep in the first place.
New regression test drives the exact failure: a registration that
fails liveness validation only surfaces its PID when probed
non-destructively; with the destructive default the standalone gateway
would be hard-killed (TerminateProcess, no drain). Mutation-checked:
reverting the probe to get_running_pid() fails the test.
Repairs #98790 where
✓ Gateway is running — cron jobs will fire automatically
PID: 4165
Ticker heartbeat: 39s ago
4 active job(s)
Next run: 2026-08-30T22:50:18.762041+03:00 in profile B incorrectly reports
that jobs will fire based on profile A's gateway process.
Root causes:
1. executed ,
which enumerated the entire systemd fleet regardless of ,
violating the docstring "only PIDs belonging to the current profile".
2. checked when
(no heartbeat file) should trigger a warning — instead, it fell through
to the "✓ Gateway is running" green branch.
Changes:
- hermes_cli/gateway.py::_get_service_pids: pattern = get_service_name()
when all_profiles=False, filtering to the current profile's systemd unit.
- hermes_cli/cron.py::cron_status: guard hb_age is None first with an
explicit yellow warning: "ticker has not reported a heartbeat".
Regression test suite guards both systemd scoping (default + all_profiles)
and heartbeat branching (None vs fresh vs stale).
Upgrades yesterday's #99310 skip-guard to full pointer-carry from
PR #90484: model assignment and custom-endpoint activation now write
key_env or the raw ${VAR} template into model config instead of
dropping the credential reference entirely, so the model entry keeps
resolving at runtime with zero plaintext in config.yaml. Applied
surgically onto current main (the PR branch predates newer
web_server.py changes); key_env carry made independent of the
expanded api_key guard, tests updated to pin pointer-carry.
POST /api/model/set copied the load_config()-resolved plaintext of a
${VAR}/key_env provider entry into model.api_key, writing the secret
into config.yaml and recreating it on every re-apply. The mirror now
checks the RAW on-disk entry and skips env-referencing entries;
literal keys keep the existing behavior.
_env_line_defines_key() decides which .env lines the writers may rewrite or
drop. It matched on the `KEY=` prefix, but load_env() splits on the first
`=` and strips the name:
key, _, value = line.partition('=')
env_vars[key.strip()] = _parse_env_value(value)
so `OPENAI_API_KEY = sk-...` is a live assignment — the key resolves, the
provider works, and every UI shows it as set. The writers did not see it.
This is the same resurrection hole #40041 fixed for `export KEY=`, still
open for the whitespace form:
- DELETE /api/env 404s ("not found in .env") while the credential stays
active — a key the user revoked through the UI is never actually revoked
- PUT /api/env appends a SECOND line instead of replacing; a later delete
removes the appended line and the original value silently comes back
Rotate-then-delete on a spaced line therefore restores exactly the key the
user rotated away from.
Match load_env()'s parse instead of prefix-matching, so the writers accept
precisely what the reader accepts: skip blank/comment/no-'=' lines, strip an
`export ` prefix, then compare the stripped name. Commented-out lines stay
untouched and `KEY_EXTRA=`/`MY_KEY=` still do not match `KEY`.
Verified against the real dashboard endpoints on a temp HERMES_HOME: the
spaced line is now removed, rotation replaces it in place with no duplicate,
and a parity check asserts the writer matches a line iff load_env() does.
`_scrub_config_yaml_mirrors` reconciles the config.yaml copies of a credential
when it is rotated or removed through the dashboard. It walks `model`,
`auxiliary.<task>`, and `custom_providers.<name>` — but not the keyed
`providers` schema.
`providers` is not a niche section: `get_compatible_custom_providers` documents
it as "the newer keyed schema" (v12+), and it is exactly where the dashboard /
desktop write a custom endpoint's inline key —
`_write_custom_endpoint` sets `providers.<id>.api_key`. That value is a real
credential: the runtime resolver reads it (`runtime_provider` /
`hermes_cli.main` / `model_switch` all read `entry.get("api_key")` off a
`providers` entry), and an inline key outranks the env var.
So the section the scrub skips is the one the dashboard writes to, and both
callers break on it:
- save_provider_env_credential (rotation, #62269): a stale
`providers.<id>.api_key` is left at the OLD value and, being
higher-precedence than the freshly-rotated env var, shadows the rotation —
the "persistent 401 with a key the UI no longer shows" that #62269 fixed,
reintroduced for the newer schema.
- remove_provider_env_credential: its contract is to "remove a credential from
EVERY store it lives in", yet the `providers` copy survives, leaving the
secret in config.yaml after the user asked to delete it.
Walk `providers.<id>` too. The scrub stays value-matched, so an unrelated
endpoint's key is untouched. Only `api_key` is scrubbed here: in the keyed
`providers` schema `api` is the base_url alias, not a credential (unlike
model/auxiliary/custom_providers), so `_fix` takes an explicit field list and
this section passes `("api_key",)` — a provider's endpoint URL is never
rewritten even if it happened to equal the credential string.
tests/hermes_cli/test_credential_lifecycle.py: drive the real PUT/DELETE
/api/env endpoints against a `providers.<id>.api_key` mirror — rotation moves
it to the new key, delete clears it, and a `providers.<id>.api` base_url alias
is preserved. The two scrub tests fail on main (stale key survives); the
base_url guard passes on main as a control. 15 pass here; 345 pass across the
credential-lifecycle + web-server suites (the one failing honcho-merge test
fails identically on clean main).
- Tavily plugin deleted (plugins/web/tavily), keyless endpoints and
ring entry removed from keyless_mcp, legacy backend set / credential
ladder / preference walks / rescue key map scrubbed.
- TAVILY_API_KEY deregistered across config, setup, status, dump, and
nous_subscription surfaces. The tvly- redaction pattern stays --
legacy keys in user envs still deserve masking.
- Sibling test pins migrated (keenable/exa stand in where tavily was
the fixture vendor); tavily test suite deleted.
- Docs updated: web-search, configuration, integrations,
environment-variables, tools-reference, web-dashboard, provider
plugin dev guide.
Live-verified from an isolated HERMES_HOME with all web creds blanked:
zero-config resolution lands in the 4-vendor ring, live keyless ring
search succeeds, no tavily anywhere in resolution order.
The PKCE payload is a flat 'provider=...;state=...;verifier=...;next=...'
string. A raw ';' is a cookie-attribute terminator, so Python's
http.cookies emits the value in RFC 6265 quoted form with each ';'
escaped as the backslash-octal '\073'. Mainstream browsers echo that
form back verbatim and Python parsers decode it — the browser round
trip is fine. But '"' and '\' are outside the plain cookie-octet set,
and non-Python hops that re-serialize the Cookie header reject the
value and drop the cookie entirely: Go's net/http (Traefik middleware,
Authentik outposts, other gateways) refuses any cookie value
containing a backslash. The OIDC callback then 400s with "Missing
PKCE state cookie" even though the browser sent the cookie.
Field reproduction: support thread "Still unable to use Authentik for
signin with traefik" — devtools showed the browser sending the intact
quoted \073 cookie on /auth/callback while Hermes logged
missing_pkce_cookie behind a Traefik+Authentik chain.
Fix: URL-encode the whole payload in set_pkce_cookie (quote(payload,
safe='') — ';' becomes '%3B') so the wire value contains only
cookie-octets and no parser in the chain has anything to reject, and
decode through a single shared inverse, cookies.parse_pkce_payload(),
in BOTH readers: the OAuth /auth/callback and the native
password-login path (routes.login_submit), whose broker/provider
binding check would otherwise parse zero segments from the
newly-encoded value and silently disable itself.
Regression coverage: the wire-shape test pins the full cookie-octet
set (the '"'/'\' assertions are the ones a Go-parser hop fails
pre-fix), the round-trip tests drive the real /auth/login →
/auth/callback path, and the next= test pins the exact post-login
redirect byte shape. Native-flow broker assertions updated to decode
through parse_pkce_payload instead of substring-matching the raw wire
value.
Salvaged from #84065 (rebased onto current main, which gained the
SameSite=None PKCE attrs and the RFC 8252 native password flow since
the PR branched): kept main's _pkce_attrs cookie shape, extended the
fix to the login_submit reader the original PR predated, and reframed
the rationale — browsers do NOT truncate at the first ';' (there is
no literal ';' on the wire in the quoted form); the failing hop is a
strict middlebox cookie parser.
Closes#83832
Co-authored-by: Kailigithub <12250313+Kailigithub@users.noreply.github.com>
SayHell0W0rld