_parallel_safe_servers was keyed by the raw server name while _servers
moved to (scope, name) connection keys (ceaf622c6d). With profile A's `x`
serial and profile B's same-named `x` opted into parallel calls, B's
discovery pass flipped A's tool to parallel-safe and the batch planner put
two A calls in one parallel segment against a server that never opted in
(review of #108352, finding B).
The opt-in is now recorded under the discovering profile's own key and
is_mcp_tool_parallel_safe() looks it up under the calling profile's key,
so one profile's policy never reaches another's same-named server.
Single-profile processes keep the bare-name key, byte for byte.
Under gateway.multiplex_profiles a profile whose mcp_servers entry has the
same route and credentials as another profile's live connection adopts that
connection instead of opening its own. _same_server_route() compares only
the connection identity, so a `trust: untrusted` profile adopted a
`trust: full` profile's connection; _trust_gate_check() then resolved the
OWNER's connection key, read `full`, and let the untrusted profile run
write-capable tools without the approval prompt its config demands
(review of #108352, finding A; regression from ceaf622c6d, where the
name-keyed ledger let the last registrant's trust win instead).
`trust` is the consuming profile's policy, not a property of the
connection: _server_trust_levels is now keyed by the calling profile's own
key (recorded at its own registration and at adoption, dropped when its
overlay is removed), while readOnlyHint stays under the connection key
because it describes the server's tools. Sharing the connection is still
allowed — only the gate is per profile.
Skipping `tests/`, `spec/`, ... in EXCLUDED_DIRS made those trees
invisible to the guard, but `plugins_loader._load_directory_module`
sets `submodule_search_locations=[plugin_dir]`, so a plugin
`__init__.py` doing `from .tests import evil` imports and runs whatever
lives there: a `tests/evil.py` with a destructive root remove scanned
`dangerous` on main and `safe` on this branch. `_walk` also matched the
names at any depth, so `src/spec/handler.py` — plain runtime code — went
unscanned.
Keep scanning everything; instead cap a critical finding located under a
ROOT-level test dir at `high`, so the verdict is `caution` (confirmation
required, `--force` overridable) rather than the un-overridable
`dangerous`. Fixture strings still cannot brick an install, which was
the reported problem, while a critical in any runtime file (`setup.sh`,
`src/spec/...`) still yields `dangerous`. Trade-off stated in the PR
body: hostile code deliberately placed under `tests/` is now
force-installable rather than blocked outright.
Docs no longer claim test code never runs.
plugin_guard walks the whole plugin clone, and EXCLUDED_DIRS skipped
caches and vendored dirs but not tests/. A security-conscious plugin's
test suite SHOULD contain adversarial fixtures — a test asserting the
trust boundary holds round-trips the injection string verbatim — and
any single critical finding makes the verdict dangerous, which --force
explicitly cannot override. Scanning tests therefore made exactly the
plugins that test their security unconditionally uninstallable, and the
only workaround was obfuscating the payload strings, weakening the
tests and inverting the incentive. Fixtures are never loaded into an
agent's context at runtime the way README/plugin.yaml are.
Add the conventional test/spec/fixture directory names to
EXCLUDED_DIRS, alongside the existing cache/vendored skips.
Both process-global caches were keyed by the caller's session/task id alone, so under
gateway.multiplex_profiles two profiles using the same id — a shared `browser_exec session=`
name, or two Hermes sessions whose screens report the same DISPLAY — resolved to the FIRST
profile's cloud browser / cua-driver, and a command issued in one bot's chat could act on
another bot's screen.
The key now carries the routed profile's home key whenever a served-profile scope is active
(`get_hermes_home_override()` set), the same shape `tools/approval.py::_baseline_key` and the
camofox/cloud caches already use; outside a scope every key is byte-identical to before. The
computer_use lookup, install and release paths all go through one `_scoped_sid`, so a release
under profile B never stops profile A's driver; approval-bypass state keeps the bare session id.
Fixes#110032 (report by @wolfyy970, from @vandaimer's manual test on #108914).
browser_exec with browser.cloud_provider: nous (the hermes tools picker row) fell into
the direct-API Browser Use branch and reported chrome-not-running, because
_resolve_backend_cdp gated on _use_gateway(), which only read the pre-picker
use_gateway: true flag. Recognize the picker selection too.
On WSL2 the only input device is the ALSA->PulseAudio bridge; with the
WSLg RDP source SUSPENDED the first InputStream.start() can exceed
PortAudio's 1 s thread-start window and fail with paTimedOut (-9987).
The failed open itself wakes the bridge, which is why the user's second
key press always worked. Retry the open exactly once when the error is a
timeout, on every platform: no WSL detection, no external parecord
warm-up. Any other error, or a second timeout, raises the same
RuntimeError as before.
Generic slim redo of #109313 by @liuhao1024 (WSL-gated parecord warm-up
and retry); diagnosis by @rugscan2021 in #109303.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
The regression only exercised register_connected_into_current_scope() and
_select_new_servers() directly. Closed#109430 covered the user-visible
path: profile B, driven through the public register_mcp_servers() entry
point, must open its own connection (its own OAuth token) instead of
adopting A's session. Fold that drive into the existing test rather than
adding a second one.
The direct _select_new_servers() assertion is dropped because it marks
B's key as connecting as a side effect, which would make the subsequent
entry-point drive skip the server; the end-to-end drive subsumes it.
check_vision_requirements (and five siblings: browser_vision, image/video
generation, x_search, browser_vault) wrapped their whole probe in
`except Exception: return False`. The registry then logged "returned False",
indistinguishable from an unconfigured backend, and the only diagnostic for a
crashed resolver was gone (#87950: named custom provider lookup failing in a
long-lived multi-profile process, reported as vision tools silently vanishing).
The registry owns the verdict: _run_check_fn_uncached and _check_fn_cached both
catch, log with traceback, and return False. Let the exception reach them.
Behaviour for the model is unchanged (tool hidden either way); agent.log now
says why.
The TTL-cached availability path collapsed a raising check_fn into the bare
verdict "check_fn X raised; dependent tools will be unavailable this turn" with
no exception attached, while the uncached path already logged exc_info. A probe
that crashes is a bug in the probe or its resolver, and without the traceback
the log reads exactly like "nothing configured" — that gap turned #87950 from a
ten-minute diagnosis into a multi-day one. Carry the exception out of the
try block and attach it to the verdict.
Trim the salvaged test to the ≤2-invariant bar: one parametrized test asserting
repeat probes stay resolvable (the user-visible contract from #87654) and that
neither a bare stub nor a Codex-adapter-wrapped stub lands under the runtime
cache key. The guard now keys on aux_probe_mode being active rather than on the
client's type, which is what covers the wrapped case.
`_store_cached_client()` refuses an `_AuxProbeClientStub`, but
`_get_cached_client()` assigns to `_client_cache` directly and so never
reaches that guard. check_fns resolve through this path inside
`aux_probe_mode()` during tool-schema assembly, and the cache key carries no
probe/runtime distinction — so the stored stub is returned to the next real
caller sharing that key, which dies on attribute access with
`_AuxProbeClientStub used as a real client (attribute 'chat')`.
The `async_mode` field in the cache key is what kept this latent: the probe
caches the sync variant, so async consumers (`analyze_image`) miss the entry
and build a real client, while sync consumers (`browser_vision`) hit the
poisoned one and fail on every call.
Observed against a local OpenAI-compatible vision endpoint, where
`browser_vision` failed every call with that RuntimeError while
`analyze_image` against the same provider worked.
Guard the inline store the same way `_store_cached_client()` does, and
return the stub to the probe caller without caching it.
The existing `test_probe_stub_never_cached` pins the invariant only on
`_store_cached_client()`, which is why the unguarded door went unnoticed;
the new test exercises `_get_cached_client()` and fails without this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Sep-2026 facade decomposition moved _detect_image_mime_type_from_bytes and
_normalize_to_supported_image into tools/vision_tools_image_prep.py; import from
the defining module per repo convention (compat pointers are off limits in-tree).
Self-review finding (hostile redteam pass), and it defeats the exact guarantee
the previous commit advertised.
The brand scan is bounded by the declared ftyp box size so a token in a FOLLOWING
box cannot upgrade a HEIC to AVIF. But the bound fell back to the whole 64-byte
sniff window whenever the declared size was outside `16 <= size <= len(header)`.
That failed OPEN in precisely the attacker-controlled case:
declared size 0 -> image/avif (genuine HEIC, 'avif' in a later box)
declared size 4 -> image/avif
declared size 8 -> image/avif
declared size 12 -> image/avif
declared size 15 -> image/avif
declared size 24 -> image/heic (truthful size, correct)
A size too small to hold any compatible brand now scans none, rather than
scanning everything. A size >= 16 that overruns the sniffed window is a truncated
read rather than an attack, so that case clamps to the bytes actually available
and a genuine oversized-ftyp AVIF is still detected.
The prior test only exercised the truthful size, so it proved the happy path
rather than the invariant it was named for. Added parameterized malformed-size
cases (0/1/4/8/12/15), a misaligned-size case (17-23, non-multiples of 4), and an
honest-oversized case. All six malformed-size assertions fail if the fail-open
bound is restored; verified by mutation, not assumed.
Two defects in the HEIF/AVIF sniffing added by this PR.
1. Major-brand-only detection mislabeled AVIF as HEIC.
AVIF encoders routinely stamp the generic still-image brand 'mif1' as the
MAJOR brand and declare the AV1 codec only in the compatible-brand list, so
every mif1-major file was reported as HEVC-coded HEIC. Parse the
compatible-brand list (bounded by the declared ftyp box size, so an 'avif'
token in a following box can't upgrade a real HEIC) and let AV1 brands win
when both families appear.
2. AVIF was rejected outright when pillow-heif was missing.
Pillow >= 11.3 bundles a native AvifImagePlugin, while pillow-heif wheels
are commonly built with no AV1 codec at all (libheif_info() reports
AVIF: ''). Gating AVIF on a pillow-heif import therefore refused files
Pillow could already decode, and pointed the user at a library that cannot
decode them. Registration is now best-effort: the decode attempt is the
arbiter, and failures emit codec-specific guidance naming the backend that
actually serves that format.
Tests: mif1+avif and mif1+av01 regressions, a box-size bound guard, a
truncated/bogus-size guard, AVIF-converts-without-pillow-heif, and AVIF error
text. Verified against real pillow-heif-encoded HEIC and Pillow-encoded AVIF
files, not just synthetic headers; both new brand tests fail if the
compatible-brand scan is reverted.
vision_analyze rejected iPhone photos with 'source is not a recognized
image'. iPhones capture HEIC (ISO-BMFF/HEVC) and upload pipelines often
mislabel it .jpg; the magic-byte sniff had no HEIF branch, so the bytes
were rejected before reaching normalization.
- _detect_image_mime_type_from_bytes: sniff the ISO-BMFF 'ftyp' box and
recognize HEIF/HEIC (heic/heix/heim/heis/hevc/hevx/mif1/msf1) and AVIF
(avif/avis) brands, returning image/heic or image/avif. Non-image ftyp
brands (mp4, ...) stay unrecognized.
- _normalize_to_supported_image: register the pillow-heif Pillow opener,
then re-encode HEIF/AVIF to PNG before embed (same soft-dependency
pattern as SVG rasterization). Missing decoder yields an actionable
'pip install pillow-heif' error, not a generic failure.
- Add pillow-heif==1.4.0 as a core dep alongside Pillow (prebuilt wheels
bundle libheif; no system libs needed).
- Tests: brand detection (incl. AVIF + mp4-not-misdetected), end-to-end
HEIC->PNG normalization, and the decoder-missing error path.
The vision resolver's magic-byte sniff stays authoritative (no extension
trust); HEIF now reaches the existing normalize step instead of 400ing.
`scripts/run_tests.sh tests/<dir>/` is how a change gets its regression
coverage run, so a test filed under the wrong directory is a test nobody
runs when that code changes. Two kinds of drift had accumulated.
Parallel directories for one source package, folded into the mirror:
tests/acp -> tests/acp_adapter (its __init__/conftest move with it)
tests/cli -> tests/hermes_cli (prompt_toolkit fixture merged into
hermes_cli/conftest.py)
tests/run_agent -> tests/agent (backoff fixture becomes
agent/conftest.py)
tests/relay -> tests/gateway/relay
tests/state -> tests/hermes_state
246 loose files at tests/ root, routed by the package they import/patch:
hermes_cli, hermes_state, agent, gateway, tools, plugins, tui_gateway, cron.
Installer and desktop-update script tests go to tests/scripts/{install,
desktop_update}/. 43 tests of root-level modules (batch_runner, utils,
hermes_constants, packaging) stay at the root.
Filenames drop their issue numbers (95 files: test_89315_x.py -> test_x.py);
the number stays in the module docstring where it has context.
Collisions: test_cli_skin_integration.py existed in both tests/ and tests/cli
with different subsets — merged into one (10 tests, all kept);
run_agent/test_pre_compress_memory_context.py -> agent/..._handoff.py;
tests/test_account_usage.py -> agent/test_account_usage_fetch.py;
tests/test_web_server.py -> hermes_cli/test_web_server_ws_ping.py.
Deleted: test_minisweagent_path.py (empty since PR #2804),
test_model_picker_scroll.py (tested a private copy of the logic, imported
nothing), test_process_loop_event_loop_warning.py (asserted asyncio behaviour,
imported nothing from Hermes).
Repo-root path arithmetic (Path(__file__).parents[N], dirname chains) is
bumped for the 202 files that changed depth and verified by evaluating every
such expression against the new location. classify_changes' desktop-updater
lane prefix, tests-os.yml's ignore glob and every in-tree path comment follow
the moves. tests/test_tests_tree_layout.py keeps the tree from drifting back.
The MCP config reconciler was appended to the gateway/run.py facade; it moves to
gateway/run_profile_reconcile.py, which already owns post-boot MCP discovery, and
run.py keeps only the chore-table entry.
reconcile_mcp_servers_with_config() also drops a schema-cache (lazy) registration
whose entry is gone (its cached tools would otherwise stay callable and spawn the
server on first use) and reports a dropped server still mid-connect as "pending";
the chore retries on the next tick without waiting for another config edit.
test_cron_delivery_housekeeping neutralizes the chore: it pins the exact
scope/drain sequence of the housekeeping loop and the new chore enters each
profile's scope once per tick.
A gateway with an OAuth MCP server whose refresh token expired opened a new
authorize tab every 300s, all night (92 tabs). Four defects stacked:
- The parked-server self-probe re-entered the SDK's authorization-code flow
with interactive OAuth enabled. The timed wake is unattended by definition:
`_wait_for_reconnect_or_shutdown` now distinguishes "self-probe" from an
explicit "reconnect", and `_park` flips the task-local
`_oauth_interactive_enabled` off before a self-probe revival.
- Gateway MCP discovery (startup, `/reload-mcp`, hot-added multiplex
profiles) ran interactive, unlike the CLI's background discovery. All three
now run under `suppress_interactive_oauth()`; an expired token parks with
the `hermes mcp login` hint instead of a browser.
- `_is_interactive()` trusted `sys.stdin.isatty()`, which the Windows CRT
reports True for a DEVNULL/detached stdin. `_stdin_is_console()` confirms
with `GetConsoleMode` on Windows.
- Removing an `mcp_servers` entry (or `enabled: false`) never reached a
running gateway; the parked server probed forever. New
`reconcile_mcp_servers_with_config()` tears down dropped/disabled servers
(via `shutdown_mcp_servers(names=...)`) and connects new ones; a
housekeeping chore runs it when config.yaml's (mtime, size) changes.
`_select_new_servers` also stops nudging disabled parked servers.
Fixes#81830. Fixes the browser-storm item of #96320.
computer_use kept its own approval decision: two module dicts
(_session_auto_approve / _always_allow) mirroring tools.approval's
session store and _persist_choice, a private verdict vocabulary
(approve_once/approve_session/always_approve) that hermes_cli mapped
back to once/session/always, and — the real problem — `if
_approval_callback is None: return None`. Only the interactive CLI ever
installed that callback, so every other host (gateway turns, cron,
api_server, tui_gateway, ACP) ran destructive desktop input with no
approval at all, ignoring cron_mode / unattended_mode / the permanent
allowlist, and "always" grants were invisible to `is_approved`,
`clear_session` and the messaging-platform approval buttons.
_request_approval now calls tools.approval._run_approval_gate with
pattern_key `cua:<action>:<background|foreground>` (the old scope shape,
so a background grant still never covers the visible foreground variant)
and fail_closed_when_no_human=True, the same posture as
request_tool_approval / the SSH-config write gate. The private dicts,
their release/atexit clearing, the verdict mapping in
hermes_cli/cli_modal_mixin.py and the extra callback install in cli.py
are deleted: the CLI's terminal_tool callback answers computer_use
prompts like any other tool. set_approval_callback stays as an optional
explicit-callback hook with the shared callback contract
(cb(command, description, **kw) -> once|session|always|deny|timeout);
no in-tree host uses it.
Behavior change:
- No approval callback and no gateway (cron, api_server/webhook,
headless -q, plain library use): destructive actions are now REFUSED
with a BLOCKED error and never reach the backend. Previously they
silently ran. cron honors approvals.cron_mode, unattended platforms
approvals.unattended_mode, -q approvals.single_query_mode.
- --yolo / gateway /yolo / approvals.mode: off still allow (unchanged).
- Gateway sessions (Telegram/Discord/Slack/...) now get a real pending
approval with once/session/always buttons instead of default-allow.
- session/always grants live in tools.approval's store; "always" is one
command_allowlist entry (`cua:click:background`) and is scoped to that
action+mode — the old blanket "always_approve unlocks everything for
the session" no longer exists.
- Denial wording is the shared gate's ("BLOCKED: User denied ...",
"BLOCKED: Action timed out ..."); the error JSON keeps `action`.
Tests: tests/tools/test_computer_use_approval_isolation.py
::test_no_callback_refuses_unless_yolo (blocked + no backend call, then
yolo executes) and ::test_always_grant_lands_in_the_shared_store
(is_approved sees the cua:<action>:<mode> key; second call served from
the store). Sabotage: restoring the `callback is None -> allow`
short-circuit fails the first; swapping the shared gate for a private
grant set fails the second plus the three delivery-ladder scope tests.
tests/tools/conftest.py gains `grant_computer_use_approvals` for
dispatch tests that only care about routing.
Seven sites hand-rolled `float(headers.get("Retry-After"))` (anon_auth,
shared_metrics_sender, gemini_native_adapter, extract_api_error_context,
nous_rate_guard, skills_hub_github, skills_hub_clawhub x2) and silently
dropped RFC 7231 HTTP-date values that the conversation loop already honours
via agent/retry_utils.py::parse_retry_after_seconds. They now call it; per-site
caps/floors stay at the call site.
The free-text "resets in / quotaResetDelay / retry after N s" regexes lived in
two tables (agent_runtime_helpers vs credential_pool) whose "resets in"
grammars diverged: the pool accepted only integer `Nhr Nmin` while the error
context accepted h/hr/hours + m/min/minutes + s/seconds with decimals. One table
(agent/retry_utils.py::RETRY_DELAY_PATTERNS / reset_delay_from_message) using
the wider grammar, so a pooled credential's cooldown and the UI's reset time
now agree.
doctor_live and kanban_decompose carried byte-identical
`try: load_config() or {}` wrappers; local_models wrapped load_config in
_quiet; each is now a direct load_config_readonly() call (read-only callers;
the canonical already fails open and returns a mapping). Tests that patched the
local wrappers patch hermes_cli.config.load_config_readonly instead.
tools/code_execution_tool._load_config read the RAW file, so a managed-pinned
`code_execution.mode` and the DEFAULT_CONFIG keys were invisible at tool
discovery — it now reads load_config_readonly() (behavior change: the managed
overlay applies to execute_code's mode/timeout). onboarding.mark_seen and
credential_lifecycle's config mirror scrub parsed config.yaml with a bare
safe_load; both are read→mutate→write round-trips and use read_user_config_raw,
the documented write-back primitive.
hermes_cli/runtime_provider._getenv was a 4-line copy of get_secret(name,
default) or default; it becomes agent.secret_scope.get_secret_str (returns
default only when the secret is genuinely unset, still raises
UnscopedSecretError — a child's unscoped read is a spawn-site bug). The
runtime_provider_backends/_custom siblings call it directly instead of via
the origin module.
tools/tts_tool, tools/transcription_tools and tools/xai_http each carried an
identical get_env_value re-export kept "so tests can patch" it; the seam is
hermes_cli.config.get_env_value, read lazily at call time. Callers
(tts_streaming, tts_tool_providers, transcription_cloud, voice_client_config,
tools_config) go there directly; resolve_provider_secret already defaults to
it so the env_getter kwarg is gone. Tests repointed at the canonical; the two
tests that only proved the shim forwarded are deleted.
Behavior change: none.
reseed_if_terminal created its temp as <auth>.rebootstrap.<pid>.tmp with
O_CREAT|O_EXCL. Boot-hook PIDs inside a container are near-deterministic,
so a run SIGKILL'd between create and replace leaves a same-named file and
every later boot hits FileExistsError - which main() swallows as
"error (ignored)", leaving the terminal-session recovery path dead until
someone deletes the temp by hand.
tempfile.mkstemp in the auth dir gives a random name at 0600 (stdlib only,
matching the script's no-hermes-imports rule); the fsync + os.replace +
unlink-on-failure semantics are unchanged.
meta/muse-image/text-to-image + paired meta/muse-image/edit, the FAL
listing of Meta's Muse Image model (launched on the Meta Model API in
Aug 2026 at $0.01/image).
- aspect_ratio size family (16:9 / 1:1 / 9:16 from the vendor's
21:9..9:21 enum); always sent on t2i for deterministic framing,
deliberately omitted on edits so Muse follows the input image.
- No seed in the vendor schema (Grok Imagine 2.0 precedent) - the
supports whitelist filters it.
- Edit takes 1-10 reference image_urls (max_reference_images=10).
Schema verified against FAL's OpenAPI for both endpoints. Live E2E
blocked by the FAL account balance lock (403), same as prior catalog
additions.
Port from earendil-works/pi#9300 fix (acaa253cc): a plugin registering a
tool whose schema["parameters"] is not a dict (a list, string, etc.)
previously registered fine and the malformed schema was serialized into
every provider request, 400-ing turns far from the offending plugin.
Live probe on main confirmed the bad schema flows into _fn_def() and the
OpenAI wire unchanged.
Fail at registry.register() with the tool name in the error instead. The
plugin loader already catches registration exceptions and marks the
plugin errored, so a broken plugin degrades gracefully rather than
breaking every session. Schemas that omit "parameters" stay valid
(no-argument tools); MCP tools are unaffected (their schemas pass
through _normalize_mcp_input_schema first, which always returns a dict).
Programs run via terminal(background=true, pty=true) can block forever when
they probe their terminal — device-status (ESC[5n), window-size (ESC[18t),
cursor-position (ESC[6n), or DEC private-mode (ESC[?N$p) queries — because
nothing on the PTY master side answers, and the raw query bytes leak into
captured output.
- tools/pty_query_responder.py: incremental byte scanner that strips the
handled queries from PTY output (chunk splits included) and produces
bounded replies; everything else passes through untouched.
- tools/process_registry.py: wire the responder into _pty_reader_loop
(POSIX only — ConPTY answers its own queries); flush partial escape
tails at end-of-stream.
- tests mirror the codex fixtures plus a live-PTY E2E where a subprocess
blocks on ESC[6n until answered.
Carve the direct-child list reconciliation from Indigo Karasu's earliest
PR #60506 (2a96ae2cbf806ccdc3e9b584911774f32622f421), corroborated by
fangliquanflq's narrow #81385 (50ffd243d92627e4a03a3ee8427ad0f5e090c3ab).
Run the existing helper after task/session filtering and reuse the
idempotent owner-stamped completion path. Do not import cross-session
disclosure, bare-PID healing, forget RPCs, or reader rewrites.
A real-child regression fails before this change and passes after it:
the direct child exits while its descendant keeps writing to stdout;
listing reports exit without consuming the result or waiting for EOF.
Co-authored-by: fangliquanflq <fangliquan@qq.com>
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
The multiplex-scoped rebase replaced the flat _permanent_baseline set with
_permanent_baseline_by_home (keyed by profile home, "" = unscoped); the
fixture must reset and seed that map.
Review follow-up. Two of the three items taken as written; the third declined
with a reason.
1. Taken. The reconcile semantics mean `patterns` may only ADD -- an entry left
out of it is not removed, because the on-disk list wins for anything this
process did not approve itself. Every caller in the tree is additive today,
so nothing breaks, but the signature does not say so. Stated in the
docstring, and pinned by
`test_a_caller_that_passes_a_smaller_set_does_not_remove` so a future
`allowlist remove` finds out here instead of in production.
NOT taken: the `reconcile: bool = True` opt-out. There is no caller that
wants it, and AGENTS.md:98-101 names exactly this -- "Speculative
infrastructure. Hooks, callbacks, or extension points with no concrete
consumer." The removal path is editing config.yaml, which the docstring now
says.
2. Taken. website/docs/user-guide/security.md, next to the existing
`hermes config edit` tip, which is where an operator reads about removing a
pattern: the list is read at startup, a pattern removed while a session is
running stays approved in that session until the next write or a restart,
and if it was removed for safety reasons, restart.
3. Taken. `test_save_failure_is_logged_not_raised` asserted non-raising but
never asserted the log its name promises. Now asserts
"Could not save allowlist" via caplog.
scripts/run_tests.sh tests/tools/test_permanent_allowlist_reconcile.py
=== Summary: 1 files, 9 tests passed, 0 failed (100% complete) in 0.4s
`load_permanent_allowlist()` runs exactly once, at module import
(tools/approval.py, the call at the bottom of the module), and
`load_permanent()` only unions into `_permanent_approved` (:2866-2869) --
nothing ever removes. `save_permanent_allowlist()` then wrote that in-memory
set straight back over `config["command_allowlist"]`, at eight call sites.
`command_allowlist` is a file the operator edits, and deleting a line from it
is the documented way to withdraw a standing approval. Any hand edit made
while a Hermes process is live was undone by that process's next `[a]lways`,
in both directions at once.
Reproduced on this tree with a temp HERMES_HOME:
BEFORE (tools/approval.py at fcbd107)
on disk before this process starts : ['git status', 'ls *']
operator edits config.yaml by hand : ['ls *', 'npm test']
(revoked 'git status', added 'npm test')
after ONE [a]lways : ['docker *', 'git status', 'ls *']
is_approved still honours revoked? : True
AFTER
after ONE [a]lways : ['docker *', 'ls *', 'npm test']
is_approved still honours revoked? : False
`npm test` was silently deleted from the operator's own config file, and
`git status` -- a standing approval they had just withdrawn -- was written
back and kept auto-approving. Neither prints anything.
The same shape loses writes between two live Hermes processes: whichever
saves second overwrites the other's entry.
The fix reconciles at write time. The file is re-read and the result is what
is on disk now, plus what this process approved since its own baseline, where
the baseline is what `command_allowlist` held the last time this process
synchronised with the file. That difference is what separates "the operator
granted this here" from "this was on disk at import and may since have been
revoked". Revoked entries are also dropped from `_permanent_approved` so
`is_approved()` stops honouring them for the rest of the process.
It does NOT make a revocation take effect the instant the file changes --
nothing re-reads the file on the approval hot path, and adding a stat there is
a separate change with its own cost. It makes the next write stop undoing the
operator's edit.
`_lock` is `threading.Lock` and not reentrant; all eight call sites were
checked and none holds it across the call, so the added critical section
cannot deadlock. The failure path still logs and returns rather than raising,
as before.
Searched open and merged PRs and issues for `command_allowlist revoke`,
`permanent allowlist reload`, `approval allowlist clobber` and
`save_permanent_allowlist` -- nothing covers this.
Tests: tests/tools/test_permanent_allowlist_reconcile.py, 8 cases -- both
halves of the bug, the two-process race, idempotence, the unedited round trip,
and the existing contract that a config write failure is logged rather than
raised.
scripts/run_tests.sh tests/tools/test_permanent_allowlist_reconcile.py
=== Summary: 1 files, 8 tests passed, 0 failed (100% complete) in 0.5s
No regression across the 29 test files in tests/ that touch the allowlist or
the approval module: 25 failed before and after, byte-identical failure set
(pre-existing missing-dependency failures in my local venv).
Port from cline/cline#13970: models that send patch calls with an empty
old_string got back 'old_string cannot be empty' — an error that names the
problem but not the recovery, so the next call was byte-identical and the
run burned turns until loop detection killed it (upstream repro: Kimi K3
looping on old_text: null).
The rejection now states the recovery: set old_string to the exact text the
replacement should replace, read the file first if unsure, use write_file
for new files/full rewrites, and do not re-send the call unchanged. The
whitespace-only rejection gets the same treatment. No behavior change for
valid calls.
The streamed download path now routes through the SSRF-safe client, which
(correctly) refuses the test fixture's 127.0.0.1 registry. Set
HERMES_ALLOW_PRIVATE_URLS for the fixture's lifetime and reset the module
cache on both sides so the guard still fail-closes everywhere else.
ClawHub ZIP downloads buffered the entire response before applying member
limits. Stream the archive into a 25 MiB bounded buffer and enforce actual
received bytes even when Content-Length is absent or incorrect.
Use the existing SSRF-safe client with bounded redirects and recheck URL and
website policy at every hop. Close responses before retry delays, clamp
Retry-After, and stop after the third rate-limited response without attempting
ZIP extraction. Preserve member path validation and raw-file fallback.
Related #29450
Co-authored-by: sprmn <oncuevtv@gmail.com>
Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
A provider whose backend keeps the response open without finishing (hanging
HTTP server, stuck SDK call) stalled the web_extract tool call — and with a
sync provider, the borrowed thread — indefinitely. The dispatch in
tools/web_tools_extract._dispatch_extract now runs under asyncio.wait_for with
web.extract_timeout (config.yaml, default 120s; 0 disables). On timeout the
tool returns structured per-URL error entries, and the one-shot keyless rescue
still gets its chance when eligible.
Salvaged from PR #57180 by @liuhao1024 (base predated the web_tools
decomposition; re-applied at the _dispatch_extract seam, env-var timeout
replaced with the web.* config section per the .env-is-for-secrets rule, and
the timeout path made rescue-aware).
Inspired by Claude Code 2.1.268: "Fixed WebFetch hanging indefinitely on a
server that keeps the response open without finishing; a fetch now fails
after 300 seconds."
Fixes#57155
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
A user message sent mid-turn (CLI busy_input_mode=interrupt, gateway priority
redirect, ACP redirect) goes through AIAgent.redirect(), which during tool
execution degrades to steer() + request_yield() on the tool worker threads.
The local terminal backend's foreground wait honours the yield (adopting the
process into the background registry), but ProcessRegistry.wait() — the
process_manage(action='wait') path — never checked it: a model sitting in a
wait on an already-background process parked the user's message for up to the
full wait window (default 180s, clamp allows more).
wait() now consumes a pending yield on its own thread each poll pass and
returns status "interrupted" with process_running=true and a note telling the
model to respond to the user; the process is untouched and still
notify-tracked. The plain-interrupt and timeout paths are unchanged.
Live repro: on origin/main, request_yield() against a thread blocked in
wait(timeout=12) had no effect (wait ran to timeout); after this change the
wait releases in <1s with status=interrupted, process still running.
Port of MoonshotAI/kimi-code#3697 ("let steer interrupt background task
waits") adapted to Hermes' per-thread yield mechanism from 463292351f.
A V4A `*** Add File:` operation is meant to create a new file. The apply
path called `write_file` unconditionally and `_validate_operations` had no
pre-check for ADD, so an Add targeting a path that already existed
overwrote the file with only the patch's `+` lines, returned success, and
emitted a `--- /dev/null` diff that hid what was lost. Models frequently
confuse Add with Update, so this destroyed existing file contents with no
error. The MOVE path already guards its destination against clobbering;
ADD now follows the same rule.
Makes a V4A `Add File` operation fail when its target already exists,
instead of silently overwriting the existing file. Validation now rejects
the operation before any write happens, so the two-phase
validate-then-apply contract ("no files were modified" on a validation
failure) holds for ADD as it already does for UPDATE/MOVE/DELETE. A
matching re-check in the apply phase closes the validate-to-apply race.
N/A
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
- `tools/patch_parser.py`: add an ADD branch in `_validate_operations`
that errors when `read_file_raw` finds an existing file, and a
defensive existence re-check in `_apply_add` before `write_file`,
mirroring the existing MOVE destination guard.
- `tests/tools/test_patch_parser.py`: add a test asserting an Add onto an
existing path fails validation and leaves the original bytes unwritten;
add `read_file_raw` to three ADD-path LSP fakes so they match the real
`file_ops` interface now exercised on ADD.
1. Build a V4A patch with `*** Add File: <path>` where `<path>` already
exists on disk.
2. Apply it via `apply_v4a_operations`. Before this change the file is
overwritten with the patch's `+` lines and the result is success;
after, the result is a validation failure and the file is untouched.
3. Run `scripts/run_tests.sh tests/tools/test_patch_parser.py` —
`TestApplyOperations::test_add_onto_existing_file_fails_and_preserves_contents`
covers the regression.
- [x] I've read the [Contributing Guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md)
- [x] My commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for [existing PRs](https://github.com/NousResearch/hermes-agent/pulls) to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: macOS 15 (Darwin 25.5.0)
- [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) per the [compatibility guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md#cross-platform-compatibility) — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
On a cloud VM the instance-metadata service hands live IAM/service-account
credentials to any local process with no auth, so a fetch against it is
credential exfiltration unless the operator expects it — yet
detect_dangerous_command() auto-approved `curl` against the link-local
metadata IP, metadata.google.internal, and the Alibaba endpoint. Add one
DANGEROUS_PATTERNS entry covering 169.254.169.254 (AWS/Azure/GCP/OpenStack),
its AWS IPv6 form fd00:ec2::254, metadata.google.internal, and Alibaba's
100.100.100.200. The host literals have no other use, so their appearance in
a command is the signal regardless of HTTP client; lookarounds keep other
169.254.x.x link-local addresses and longer host/dotted strings out.
This prompts for approval (legit uses exist on real cloud VMs); it is NOT a
hardline block. Deterministic containment-escape detection at the approval
layer, same class as the existing credential-path detectors.
Relocated onto the decomposed module layout and hardened:
- Trigger covers the rejection CLASS, not just literal 400: SSE-only
servers' load balancers answer the chunked Streamable HTTP initialize
POST with 400/405/406/411, and the mcp>=2.0 SDK surfaces many such
rejections as an opaque -32603 'Server returned an error response'
(error class per #104363 by @RohithPariki). Timeouts and 5xx never
trigger the fallback: they are not transport mismatches.
- Reconnect exclusion via _ever_connected instead of _ready: run()
clears _ready before re-entering the transport, so the original guard
also fired on reconnects after a proven session.
- Successful fallback latches _sse_fallback so reconnects go straight
to SSE, and logs a warning suggesting the user pin transport: sse.
- Both transports failing raises a ConnectionError naming both errors
and suggesting transport: sse / checking the URL.
- No fallback with strict_redirect_headers (SSE cannot enforce that
boundary) or when transport is explicitly configured.
- Tests trimmed to 3 invariant contracts (proven red on base): fallback
connects + latches; no fallback on reconnect/timeout/5xx; both-fail
error is actionable.
The extracted SSE path reuses _sse_transport/_serve_transport from main,
preserving the bounded handshake timeout and reconnect-retry semantics.
Fixes#53676
SSE-only MCP servers (e.g. WigAI for Bitwig Studio) reject the
Streamable HTTP initialize request with 400 Bad Request, causing
permanent failure with 0 active tools. The only workaround was
manually setting transport: sse in config.
When Streamable HTTP returns 400 during initial connect, log a
warning and retry with SSE before reporting failure. Reconnects
are excluded so a genuine 400 on an established transport is not
silently masked.
Extracted inline SSE code into _run_sse() helper shared by the
explicit config path and the new fallback path.
Fixes#53676
Unpinned cron jobs snapshot the global provider/model at creation and fail
closed when the global default drifts (#44585). Pinning was the only way
forward, but it makes a job stop tracking the global default forever.
Add resnap: refresh an unpinned job's provider/model snapshot to the CURRENT
global resolution without pinning it, so it adopts the user's deliberately
changed default while keeping tracking future changes. Single job via
cronjob(action='resnap', job_id=...) or hermes cron resnap <id>; bulk via
cronjob(action='resnap', all=true) or hermes cron resnap --all. Refuses to
guess scope when neither is given. The drift-guard alert now points at both
options (pin vs resnap). No inference call is made — it recomputes the
snapshot string from config.
When a persistent Docker container is removed out-of-band or a Vercel
sandbox hits a terminal state, the backend silently recreates it and
retries. The model then keeps assuming background processes and
non-persisted files from earlier commands still exist.
Backends now call _mark_recreated() after a successful recovery;
BaseEnvironment.execute() folds the one-shot flag into the result as
environment_recreated, and finalize_foreground_result() attaches a
model-facing warning field explaining what may have been lost.
Ported from lobehub/lobehub#19329 (sandbox recreation surfacing),
adapted to hermes environment backends and tool-result JSON.
Two long-standing, mutually-masking defects in read_file's line accounting,
present on all three read paths (compound shell probe, sequential probes,
native):
1. total_lines came from `wc -l`, which counts newline bytes: a file whose
final line has no trailing newline was undercounted by one. For an
N*limit+1-line file read in pages, the last page was never offered
(truncated=False) and the past-EOF guard refused `offset=total` reads of
the real final line.
2. _add_line_numbers() split on '\n' without dropping the single terminating
newline, so every well-formed newline-terminated file rendered a phantom
`N+1|` empty gutter line that does not exist in the file (models routinely
tried to patch/reference it). Exactly one terminator is dropped, so a
genuinely selected trailing blank line keeps its number (`cat -n`
semantics).
Both fixes land at the shared choke point (_assemble_read_result /
_add_line_numbers) so the compound, sequential, and native paths agree.
Existing tests that froze the buggy rendering as expected output are updated
to the corrected contract.
Salvages community PRs #106888 (@nikkoxgonzales) and #49453
(@MaxFreedomPollard); same bug class independently reported/fixed in
#3907/#3908, #3927, #20814, #22945, #42929, #55696, #91306.
Cross-validated against anomalyco/opencode#47420's read-page serialization
fix (their trailing-blank-line class; hermes' page path is already
blank-line-safe once the terminator handling is right — verified live).
_add_line_numbers split on '\n', so a file ending in a newline (the normal,
well-formed case) produced a trailing empty element that got its own line
number. read_file therefore showed a phantom '<N+1>|' line that is not in the
file, on every terminal backend and every OS, matching neither cat -n nor the
reported total_lines. Drop the single terminating newline before splitting.
Fixes#49451
wc -l counts newlines, not lines, so a file without a trailing newline
reported one fewer total_lines than the content it returned. The read
paths already probe the last byte (file_ends_with_newline) to strip cut's
phantom newline; use that same signal at the shared assembler choke point
so total_lines, truncation, and the past-EOF guard agree on every path
(compound, sequential, native).
Fixes#3907. Supersedes #3908: single adjustment instead of a per-path
helper, covering the native and sequential paths as well.
teknium1