Gate r1 quality finding: test_empty_ghost_session_does_not_block_title
read as "ghosts never block", but three of its four rows assert the
opposite. Rename it to
test_title_conflict_yields_only_to_ended_empty_visible_holder, make the
holder_messages count a holder_has_message bool (only zero vs non-zero
matters), and state the rule in the docstring: a holder yields only when
ended, empty, visible and unarchived; every other holder conflicts.
Gate r1 2c finding (same bug class as #81888): _handle_create_session ran
its own "SELECT id FROM sessions WHERE title = ?" and 400'd whenever any
row held the title, so creating a session named after an abandoned empty
chat still failed even though renaming to that name now succeeds.
Extract the conflict block of _set_session_title into
SessionTitlesMixin._resolve_title_conflict(conn, session_id, title) and
call it from both the rename path and the API create transaction, so the
uniqueness rule (compression ancestor transfer, retired Bot Chat, ended
empty visible ghost, otherwise ValueError) lives in one place. On
ValueError the create still deletes its just-inserted row and returns
400 invalid_title; the message is now the DB's "Title 'X' is already in
use by session Y". A brand-new session has no compression ancestors, so
that branch is a no-op on create. api_server.py net growth is 0.
Filtering the conflict check with ``message_count > 0`` let two sessions
hold one title in the DB: the full unique index then raised
IntegrityError, and under the PR's partial ``message_count > 0`` index the
ghost's first append_message would fail instead. Keep the full index and,
in the same CAS transaction (like the archived Bot Chat branch), retire
the title from a visible row matching _EMPTY_SESSION_WHERE: ended, no
messages, not archived -- the same definition delete_empty_sessions uses.
Live empty rows (a /title queued before the first turn), hidden rows (a
fresh canonical Bot Chat) and archived rows keep their names.
Empty sessions (message_count=0) created by interrupted flows (e.g.,
desktop session creation that never received a message) would block
session rename attempts even though they were invisible in the UI.
Changes:
- Updated title uniqueness index to exclude sessions with 0 messages
- Modified _set_session_title conflict check to skip empty sessions
- Added migration to drop and rebuild the index with new filter
- Added regression test for ghost session rename blocking
Fixes#81888
(cherry picked from commit a28e084278694a3b7e5f0adbfae90b14ac575db2)
When every DRM card's vendor file or every Windows display adapter's
MatchingDeviceId was unreadable, classify_gpu_vendors returned 'none' and
availability refused with unsupported_gpu, contradicting the documented
'unreadable GPU passes' rule. Windows now records an unreadable numbered
adapter as an empty id so the classifier sees it.
Also drops the type-narrowing assert in availability() and makes
_action's gpu_label required.
A portable plugin can now declare `requires: {gpu: nvidia}` for an MCP
server. On a host without an NVIDIA GPU, Hermes then refuses the
install, hides the server's tools, and the catalog card says "needs an
NVIDIA GPU". The check is the existing
`hermes_platform.host.facts.gpu_class()` read.
The requirement is independent of `app:`. The RTX Remix Toolkit is the
first consumer: it is a package the user unpacks anywhere, so there is
no install location to check, but it needs an NVIDIA GPU.
An unreadable GPU (`unknown`) passes, so a failed registry or sysfs
read does not lock out a machine that has the GPU. Only `nvidia` is
accepted: gpu_class() reports the highest-priority vendor, which
answers the question exactly for NVIDIA and not for AMD or Intel.
A registered server on such a host reports the new `unsupported_gpu`
state, carried through the gateway contract and the Plugins tab, with
a sentence that names the GPU instead of "Install <app>".
README rule 9 (#130795): listed plugins extend Hermes only through public
surfaces. These 11 entries rebind core functions/methods/module tables or
reach into the Desktop app's own markup, and fail the no core override /
app DOM reach admission checks at their pinned shas.
Plain delisting, not removed.yaml: installed copies keep working, and each
plugin is welcome back with a SHA bump once it moves to a public hook. Every
author has an issue on their repo explaining the finding and the public path.
Dropped from the original list because their compliant re-pins were
approved: clinepass (#130439), prompt-enhance (#130900),
custom-dangerous-patterns (#131920), hermes-zh (#131464) and
skill-retrieval (#131290).
plugins.isolation is profile config, so a profile (or its agent) can set it
back to in_process. The managed scope already beats profile config and makes
`hermes config set` refuse, and the isolation reader goes through that
overlay; document it as the operator lock and pin the contract with a test
(red when isolation_mode() reads the raw profile file).
The credential-free profile extraction child got HOME but not USERPROFILE.
Windows resolves Path.home() from USERPROFILE only, so the child exited with
"Could not determine home directory" and every user model-provider plugin was
silently dropped under host isolation.
Reported-by: Enough1122
Independent review of the plugin host found eight defects; each is fixed here and
the reproducible ones are covered by two new invariant tests (red on the previous
head, green now).
- Async plugin code: `await ctx.<facade>.<async method>` works (the facade probe
reports coroutine methods and the host returns a coroutine function), and ctx
calls made from coroutines keep the caller's session. The wire origin is now a
ContextVar carried onto host event-loop tasks, and the host's fallback context
starts empty instead of copying whichever caller created the host.
- Memory / context-engine / cron providers survive a host crash: proxies record
the host generation, and a category proxy reloads its plugin into the new host
on next use (bounded by the same restart budget). A proxy from an earlier host
can no longer address a recycled object id.
- Hosted provider objects no longer leak: the host imports a category plugin's
module once and drops objects whose Hermes proxies were collected.
- Every public attribute of a hosted object is read live (class-level defaults
included), and attributes Hermes assigns reach the plugin's object unless they
hold a live handle.
- A user memory provider's config_schema.py runs in the plugin host.
- Dashboard plugin APIs resolve the plugin in the requesting profile (its copy,
its plugins.enabled, its host) and keep repeated headers such as Set-Cookie.
- platform_actions coroutines run on the gateway loop their adapters use.
- Each incoming request gets its own thread (no pool exhaustion under nested
callbacks); a late close from a replaced host no longer triggers a reload; the
model-provider extraction cache is keyed by full path.
plugins.isolation: host imports every non-bundled plugin in a host process started
on demand per profile with only that profile's environment, and hands plugins a
RemotePluginContext; Hermes registers proxies for tools, hooks, commands, skills and
provider objects (subclasses of the ABC it checks). Re-entrant: handlers may call
ctx back into Hermes while Hermes waits on them. Every user-code import path routes
to the host (general, memory/context-engine/cron, model-provider profiles via a
cached credential-free extraction, dashboard APIs via an ASGI bridge) or refuses.
One table names what cannot cross (live gateway objects); the static audit shown
by `hermes plugins validate/show` reads it. A host that dies is restarted with its
plugins (bounded). Default stays in_process: nothing changes unless opted in.
Why: shared-pool deployments must never run one tenant's plugin Python in an
interpreter holding another tenant's data, without plugins becoming tenant-aware.
The worker isolated HERMES_HOME only. The longrange_backup_pipeline task
("schedule it nightly at 2am") let agents run the operator's real crontab
and write into the real ~/.hermes: a past battery left three nightly cron
jobs and two scripts behind on the host, pointing into deleted run dirs.
sandbox.isolate_host turns on the product's own TERMINAL_HOME_MODE=profile
contract, points the worker's HOME at the same profile home (in-process
~ expansion for write_file), puts a file-backed crontab shim first on PATH
and unsets the systemd user bus. README quickstart drops its /tmp paths.
The scheduler persists last_delivery_unverified as a list of unconfirmed
platform:chat_id[:thread_id] targets and cron.manage passes it through, but
CronJobRow typed it bool | None, so contract validation rejected any job
with an unconfirmed delivery. Regenerate the TS/OpenRPC contract.
Follow-up to #131853 and #131867 from an independent review, each finding
reproduced in native Electron against two real serve backends.
- Cold composer path: the cache reconciliation (mergeServerMeta) ran only in
the Bots pane's publish effect, so a launch that never opened the pane kept
a stale cached title ahead of the backend's own in @handles and group
prompts ("You are @local-stale-name"). It now runs where every snapshot is
produced (data.ts::fetchRosterSnapshot), with the same fetch-time write
fence, so the pane query and primeRoster reconcile alike.
- Pane label vs @handle: displayName ranked display_name above the backend's
Bot Mode title on a thin remote row (and a titled remote `default` with no
display_name fell to the connection label), so the row read "Remote Human
Name" while mentions and prompts said @remote-bot-title. It now takes the
same backend-title rung botFriendlyNames uses.
- Relay trace: a connection removed and re-added under a new id was logged
`MISROUTED (same install as <removed id>)`; the remembered owner is now
pruned against the current connection set.
The roster-pane-lifecycle test probed "republished" via mergeServerMeta's call
count; reconciliation now happens per fetch by design, so that invariant is
asserted via trackInboundActivity alone.
A `GOCSPX-` literal is the client secret of a Google installed-app (desktop/CLI)
OAuth client. Google's OAuth docs state such secrets cannot be kept confidential:
gcloud ships its own in every copy. The generic
`hardcoded_secret` pattern scored it critical, so any plugin that talks to a
Google installed-app client (the catalog's quota plugin) scanned `dangerous` and
could not be installed even with --force. Cap that one shape at high: the
finding stays in the report and the install asks for review (caution); every
other secret-shaped literal keeps its critical severity.
Salvage of #131963 by @mekenthompson onto main after #131946.
The shutdown reaper's SIGKILL pass only checked whether the leader PID was
still alive. Once the leader had exited, a descendant that ignored SIGTERM
kept the process group alive and was never escalated. The pass now also
probes the group with killpg(pgid, 0) and sends SIGKILL through the same
start-time guard as the SIGTERM pass.
The per-spawn environment marker from #131963 is not carried over: it left
helpers launched with a scrubbed environment running on Linux and brought
back the dead-leader refusal on macOS when a start time was recorded.
Reverts the nine commits merged by #132260 (merge 05ab8766e7), from
31194ea00e "fix(cli): revalidate worktree cleanup gates before removal"
through 05ab8766e7, restoring hermes_cli/worktree_gc.py,
hermes_cli/worktree_ops.py and tests/security/test_gitspawn_config_injection.py
to their pre-merge state and removing
tests/hermes_cli/test_worktree_reclaim_revalidation.py.
Maintainer decision: back the behaviour change out (trees kept when their
state changed since the audit, plain `git worktree remove` under the tree's
git env, dirty submodule trees kept). No commit on main touched these files
after the merge, so this is the exact inverse of the merged diff.
The contributor mapping for @rudycelekli stays (attribution record only).
Review finding (BR1 r3 efficiency, Low): the _ENCODED_URL_PARAM_RE key
group excluded only `%3D`, so a key could run across `%26`/`%3F`/`%23`/
`%3B`. Two consequences: a nested `?next=%2Fx%3Ffoo%26token%3D<secret>`
matched the key `foo%26token`, which is not a sensitive name, and the
secret leaked; and each separator restarted a match that scanned up to
160 chars, about 8x slower than base on 1 MiB of `%26a` (1.72s vs 0.21s;
0.36s now).
Exclude every encoded separator from the key so it ends at the nearest
one. The nested leak now redacts to `%3D%2A%2A%2A`, and the
`redirect=...%3Fapi_key%3D...` case is unchanged.
Review finding (BR1 r3 2c Medium / 2ab Low): the `***` arm added to
_BARE_HEADER_RE treated ANY same-line word after an already-masked
value as the secret, so benign prose was eaten and every re-scrub ate
one more word (`x-api-key: *** see docs for details` -> `... docs for
details` -> `... details`; same for `Authorization: *** then prose`,
`X-Api-Key: *** (set in env)`). The scrub was not a fixed point.
Require the follow-on value to look like a credential (>=8 token chars,
optional `=` padding, then a token boundary) before `***` counts as a
masked scheme. The floor is 8, not 16, so 15-char keys still redact.
The 336-case header sweep stays at 0 leaks / 0 unstable, the 18
`name: <scheme> <key>` cases are still redacted, and the benign lines
are stable over three passes.
agent.redact's _SECRET_HEADER_RE treats the first word after an API-key
header as the value. With `x-api-key: Basic <key>`, that word is the scheme,
so the scheme becomes `***` and the key is left in the text. The support scrub
then saw `***` as the value and stopped, so `x-api-key: *** <key>` reached the
debug/share upload with the raw key. This affects the x-api-key, api-key and
x-auth-token families with Bearer/Basic/Digest, in both bare and `-H '...'`
forms. Base and main leak the same input.
_BARE_HEADER_RE now also accepts `***` plus a same-line value as a scheme.
That value must not be another secret header, so `x-api-key: *** cookie=...`
is not consumed. The key is redacted, and the redacted output is a fixed
point. The fix stays in the support layer. Teaching agent.redact the scheme
does not help, because its YAML/assignment passes mask the scheme word first.
Gate finding: BR1 fold2 "still open", a bare-header scheme leak.
In `(?>(?:Bearer|Basic|Digest)\s+)?` the `?` sat outside the atomic group, so
the engine could skip the group and let the `(?!\[REDACTED\])` lookahead pass
on the scheme word itself: re-scrubbing `--header Authorization Bearer
[REDACTED]` gave `--header Authorization [REDACTED] [REDACTED]`. Moving the
optional quantifier inside the atomic group makes the scheme consumed whenever
present, so already-redacted values are a fixed point.
Gate finding: BR1 r2 2c (Low), scheme idempotence.
The 2x-cap window only cut at the first newline after 2*cap, so a client value
with no newline (one 2 MB line) was still scrubbed whole: the superlinear strict
scrub ran on megabytes from an unbounded RPC param. Look for the line end only
in [2x, 3x) cap and fall back to a hard 2x-cap cut, so the scrubbed window is
always <= 3x cap while a key cut by the cap still gets redacted.
Gate finding: BR1 r2 2c (Medium) / r2 2ab (Low), convergent.
Quality review (Low): _read_tail_bytes only returns a non-zero offset
when size > max_bytes, so start_offset was redundant in the truncated
flag. In diagnostics.share_nous the except path re-imported
debug_redaction lazily; if that import was what failed, the handler
escaped instead of returning a structured error. Import it once above
the try and use it on both paths.
Reuse review (Low): _decoded_compact_name re-implemented
agent.redact._canonical_url_param_name (3-round unquote_plus + casefold).
Compacting the canonical name is equivalent (0 mismatches over the
sensitive list + 20k random %-nested/+/-/_ names), so build on it and
drop the local loop and unquote_plus import.
Gate review (Low, convergent 2c/efficiency/quality/reuse):
_SENSITIVE_HEADER_COMPACT_NAMES lost its only reader with the removed
structured walker, and _redact_log_text / redact_debug_support_error
were pure pass-throughs to redact_debug_support_text. Delete all three
and call redact_debug_support_text at every site (debug.py, dashboard
status router, /debug slash command, share_nous RPC); test docstrings
now name the real function.
Gate 2c + quality review (Low, convergent): the header/argv value
classes matched "[REDACTED" inside an already-scrubbed value, so a
second pass rendered "cookie=[REDACTED]]". That second pass is real:
upload_to_pastebin scrubbed each per-service error and every caller
(debug share, dashboard, /debug) scrubbed the wrapping RuntimeError
again. Skip already-redacted values with a (?!\[REDACTED\]) lookahead
in the four value-group regexes (scheme prefix made atomic so it cannot
backtrack around the guard), and drop the inner scrub in
upload_to_pastebin since all callers redact the combined error.
Efficiency review (Medium): redacting the whole client-supplied
error_context / extra_files value before capping made the scrub cost
grow superlinearly with an unbounded RPC param (2 MB -> seconds on one
RPC thread), where main capped first. Scrub a line-aligned window of
twice the cap instead: a key cut by the cap is still redacted, the
capped output is identical (2 MB probe: same bytes, max scrub input
2 MB -> 1 MB), and the cost is bounded.
Wrapping every share_to_nous failure in RuntimeError(redacted text)
erased the exception type for callers. Every caller already renders
the error through redact_debug_support_error, so the wrap added no
protection; leave the types as they were.
redact_debug_support_text ran redact_for_egress (itself a forced
redact_sensitive_text pass) and then a second forced pass with URL
credentials; collect_share_bundle then re-scrubbed every bundle member
and build_nous_bundle walked it a third time. A ~1MB log took ~1.0s vs
0.32s for one pass, per log. Run redact_sensitive_text once (the
support bearer sweep already covers redact_for_egress's residue
sweep), drop the bundle re-scrub and the Nous re-walk (every member is
already scrubbed at capture), and with them the now-unused structured
value walker. Also drop _redact_log_text's ignored
redact_url_credentials kwarg and fail closed with the shared
REDACTION_UNAVAILABLE sentinel.
The support scrub re-listed agent.redact's sensitive query params and
secret header names; two copies of one pattern list drift. Derive the
support-only compact names from _SENSITIVE_QUERY_PARAMS (plus the five
support-only extras) and expose the header names as a tuple that both
the agent regex and the support module build from. agent.redact is a
hard dependency, so import it at module level and drop the try/except
around is_secret_field_name.
The HEADER.FIELDS list hand-copied the _AUTOMATED_HEADERS keys, so a key
added there would silently miss the preflight and the sender's full body
would be fetched before being dropped. Build it from the registry (the
IMAP command is byte-identical today). The preflight also repeated the
RFC822 payload unwrap and had three identical warn/consume/continue
branches: one _imap_payload helper now serves both unwraps and the
rejections collapse to one branch (gate reuse + quality Low, convergent).
preauthorize=None kept an unsafe path on the shipped adapter: any caller
that left the gate out still RFC822-fetched unauthorized mail. The only
production caller (_check_inbox) always passes it; the None default served
only direct test calls (gate quality Medium + 2c Low, convergent). Make it
required, drop the guard, and have those tests pass an allow-all gate
(their IMAP mocks answer the BODY.PEEK preflight where needed).
The gate hand-rolled run_coroutine_threadsafe(...).result(). On a closed
loop that raised and leaked a never-awaited coroutine (gate reuse Low
finding). The shared helper closes the coroutine and returns None, which
the gate now treats as a rejection, so it still fails closed.
Every topmost row carried dmarc=fail, so it failed on its verdict no
matter what the pin check did: restoring subdomain alignment on the
topmost field kept the test green (gate 2ab Low finding). The subdomain
and comment-smuggled rows now say dmarc=pass, so only the exact pin
rejects them and that mutation goes red.
Every sibling signature in the adapter spells the type through the
existing email_lib import; the bare Message import served one annotation
(gate quality Low finding).
The authserv_id pin now matches the topmost Authentication-Results host
exactly; main also accepted a subdomain of the pin. Operators who pinned a
parent domain for a provider that stamps from changing MX hostnames would
see every message dropped with no guidance (gate 2c Medium finding).
The adapter now trusts only the topmost Authentication-Results field and
requires a set authserv_id to match it exactly (no subdomain alignment),
so operators whose relay stamps a different top authserv-id see the
untrusted-authserv warning. Say so where the warning is documented.
The adapter fetched the full RFC822 payload (and decoded attachments) for
every unseen UID before deciding whether the sender was allowed, and
_verify_sender_authentication searched every Authentication-Results field
for one whose authserv-id matched (or was a subdomain of) the pin. A
forged lower field or an attacker-chosen child.<authserv_id> therefore
authenticated a spoofed From:.
- Only the topmost Authentication-Results field is authoritative; a set
authserv_id must match it exactly (RFC 8601 CFWS comments allowed via
_ar_clauses). Main's _NO_AUTH_RESULTS_REASON/_UNTRUSTED_AUTHSERV_REASON
constants are kept so _DROP_HINTS still resolves.
- _check_inbox fetches a bounded BODY.PEEK[HEADER.FIELDS ...] preflight
and runs _sender_accepted on the event loop before requesting RFC822;
rejected UIDs are marked \Seen without downloading the body.
Salvaged from #129772 and rebuilt on main: the PR skipped the preflight
under open access via self._open_access(), which main removed in
c2f595d6af (open access now also authenticates From:), so the gate always
runs here. Poll-loop test mocks now authorize their unauthenticated test
senders instead of forcing open access.
reclaim_worktrees loaded the git-cherry merge cache on every call, dry
runs included, though a dry run never revalidates. It also never saved
the verdicts revalidation added, so the next audit re-ran `git cherry`
for any tree whose head moved after the audit (gate r2 efficiency, Low).
Load it only for a real run and save it once after the loop when it
grew, as audit_worktrees already does. Verdicts are unchanged: the cache
is only a memo.
The attended reclaim's plain `worktree remove` (no --force) reads the
linked tree's index, so it re-hashes stat-dirty files through clean
filters. It ran under noninteractive_repo_git_env(repo_root), which only
discovers filters visible from the main checkout: a filter defined in the
tree's config.worktree (extensions.worktreeConfig) ran its command
(gate r2 2c, Medium). Build the env from the tree itself, as the startup
path already does, keep the tree when discovery fails, and run both the
plain remove and the submodule --force retry under it.
The security test now plants a tree-scoped clean filter on the trees the
startup sweep and the attended reclaim remove.
Gate finding (Low, quality): the "state changed since audit" message
printed only the fresh classification reason. When the branch or the
untracked list had changed, the verdict was still reap, so the message
read "state changed since audit: clean and fully merged/pushed", which
explains nothing. It now names the field that changed (verdict /
untracked files / branch) and gives the current reason. Also fixes the
stale "`remove --force` refuses" comment: removal is now usually a plain
`worktree remove`. The dead `if not refreshed` guard was dropped in the
previous commit.
