search_files over $HERMES_HOME returned the token from backups/config/config.yaml.good.<stamp>
in cleartext while config.yaml itself was masked; the predicate only matched the exact basename.
Same predicate serves the terminal side, so `cat backups/config/config.yaml.good.*` is covered too.
Five tests instead of fourteen: one parametrised table proves every assignment
shape (YAML, shell rc, dotenv, JSON, read_file and grep -n gutters, vendor
prefix) masks to the non-reusable sentinel with key/gutter/quoting intact; one
proves unclassified reads and non-secret scalars are byte-identical; one pins
the resolved-home classification (Windows home without a .hermes segment) plus
the project-config negative; the two tool-level tests drive read_file and
search_files through the real call sites, with the project config.yaml
negative folded into the read test.
read_file/search_files passed file_read=True, which folded into code_file=True and skipped
the ENV/JSON/YAML assignment passes, so an opaque prefix-less credential under a
credential-shaped key reached the model in cleartext from a secret-bearing file — the
file-read half of the #110228 gate (#110567).
Two defects on that path, both fixed here:
- The rendered line-number gutter ("5| ADS_API_TOKEN: ..." from read_file,
"6: ADS_API_TOKEN: ..." from grep -n / cat -n) defeated the line-anchored patterns,
so the real rendered read leaked exactly what the raw text masked. A gutter-free fixture
cannot see this, which is why the tool-level tests carry the real render shape.
- _is_secret_file_arg() could not see the RESOLVED Hermes home: the default home's basename
is an installation detail (".hermes" on POSIX, "hermes" under AppData/Local on Windows) and
a resolved path never spells $HERMES_HOME, so the managed Windows home's config.yaml was
classified as ordinary YAML on both the file-read and the terminal surface.
Changes:
- redact_sensitive_text(): secret_file= re-enables the assignment passes for content the
caller classified with _is_secret_file_arg, keeping code_file behaviour everywhere else.
It is authoritative over code_file, so a caller cannot be fail-open on the security flag
by setting both.
- _redact_assignments(): mask_nonreusable selects the non-reusable sentinel for file reads,
so the #35519 write-back hazard stays closed.
- _should_redact_assignment(): no longer re-masks an already-masked value, which was erasing
the vendor label the sentinel deliberately keeps.
- _is_secret_file_arg(): consult the resolved Hermes home for the config.yaml arm.
- _CFG_ANCHORED_RE / _YAML_ASSIGN_RE: tolerate a rendered line-number gutter.
- file_tools.py: classify the resolved path at all three file-read call sites.
Closes#110567
create_task(parents=[open parent]) — the reporter's actual incident path —
parked the card in todo with only a `created` event, and kanban_create's
payload carried no `gated`, so the board still showed an unexplained todo
while only the link surface was fixed. create_task now appends the same
dependency_wait {reason: parent_not_done, parent} event and kanban_create
returns gated/gated_by, mirroring kanban_link.
link_tasks gated on `status != 'done'`, but _parents_satisfied and
recompute_ready treat `archived` as terminal: linking a ready child under an
archived parent demoted it to todo with a false parent_not_done event and the
next recompute promoted it straight back. Gate on not in ('done','archived').
Review finding: create_task(parents=...) emitted no dependency_wait/gated; link under an archived parent flapped ready->todo->ready with a false reason.
The dashboard's POST /links is the fourth writer of link_tasks (CLI, tool,
dashboard, plus the graph builder); return the same ``gated`` flag so every
surface that can create the deadlock can see it. Document the
``dependency_wait`` payload the link path emits and the delegation rule the
reporter derived (never link a support card under the card it unblocks).
Drops the CLI output test (a change-detector on prose); the two DB-level
invariants (event emitted on demotion / none for a done parent) stay.
A ready child linked under an unfinished parent drops to todo with no
event and no operator signal; the only trace used to be claim_rejected
after a forced promote. Record a dependency_wait event when the demotion
fires, return the gate from link_tasks, warn in the CLI link command,
report gated in the kanban_link tool, and document the gate.
The per-root merge in _copy_dist_payload treated the first level under
skills/ as the replace unit, but skills live at skills/<category>/<skill>.
A distribution shipping one skill inside a category rmtree'd the whole
category directory, wiping every skill the user (or hermes update) had
placed there — the exact scenario of issue #25120 that this PR claims to
fix. A shipped directory with no files of its own is now a container:
its children are merged one level down, and the replace unit becomes the
nearest directory that holds a file (a skill dir always holds SKILL.md).
A symlinked category (skills/devops -> shared dir) is a container too,
so the pre-write symlink check now covers it instead of silently
unlinking the link and dropping the shipped copy in its place.
Review finding: categorised skill payload rmtree'd skills/<category>/,
destroying sibling user and bundled skills; symlinked category was
unlinked rather than refused.
The user guide, command reference, `hermes profile update --help` and the
install-over-plain-profile warning all said skills/ and cron/ are overwritten
wholesale; they are now merged per skill / cron job, so say so, and document the
symlinked-container refusal.
Fold the skills/cron/force-install cases into one per-root merge test and keep the
symlink-refusal test; the three separate tests asserted the same mechanism.
The symlink refusal lived in `_real_dir`, which runs per entry inside the copy
loop. By the time it fired for `skills/`, `SOUL.md`/`mcp.json`/`cron` had already
been replaced and `write_manifest` had not run yet, leaving a half-updated
profile that fails identically on every retry. Add a pre-flight pass over
`_owned_entries` that walks each target path (whole chain for directories,
parent chain for files) and raises before any mutation; `_real_dir` keeps its
check as the backstop.
The `.env.template` branch used a bare `shutil.copy2`, which writes through an
existing `.env.EXAMPLE` symlink into whatever it points at. Route it through
`_replace_entry` so the link is unlinked first, like every other owned entry.
The error text now tells the user what they can actually do (remove the link or
replace it with a real directory) instead of pointing at `distribution_owned`,
which is the author's manifest, not the user's.
The existing symlink test now changes every other shipped entry upstream and
asserts the profile is byte-for-byte untouched after the refusal.
A user who points `profiles/x/skills` at a shared directory did so on
purpose. The merge path replaced such a symlink with a real directory
silently, discarding that configuration, where the base rmtree at least
failed loudly. Raise DistributionError naming the link instead; the
file->directory replacement stays because a stray file there is never
deliberate configuration.
`_remove_existing` now removes anything that lexists but is not a real
directory via unlink, so fifos and sockets no longer slip through.
Reviewer P1 on the original PR (symlink container must survive intact).
Keep the tests that pin behaviour a regression would break: user-added and
stale skill roots survive an update while shipped roots are replaced, a
forced reinstall keeps a user skill, and a user-added cron file survives an
update. The allowlist and symlink-transition cases exercise the same code
path (`_real_dir` + `_replace_entry`) and were duplicates of those.
Refs #110933
The skills-only special case (a `preserve_skills` kwarg plus nested closures
keyed on the literal "skills") left cron/ and any future owned directory on
the old rmtree-then-copytree path, which deletes user-added files on update
and on --force reinstall. The rule is the same for all of them: a top-level
owned directory is a container of roots; replace only the roots the payload
ships, never the whole destination.
Apply that rule to every owned directory, drop the kwarg and the string
check, and lift the helpers to module level. `_remove_existing` keeps its
symlink-no-follow semantics and `_real_dir` replaces any symlink or file on
the destination path so the payload is never written outside the profile.
The fresh-install path had nothing to preserve, so it needs no flag.
Restore "skills" in DEFAULT_DIST_OWNED; removing it was never needed for the
fix. Add one invariant test: a user-added cron/ file survives an update.
Refs #110933
_BLOCKED_PROJECT_ENV_BASENAMES is consulted only by get_read_block_error;
project .env / .env.local / .envrc are read-denied but writable on both main
and this branch, so the write-safety table row saying "anywhere on disk" was
wrong. Denying writes would stop the agent from creating .env files for the
user, so the doc is corrected rather than the behaviour changed.
Review finding: security.md write-safety table listed project .env files that are only read-denied.
The salvaged fix added a second directory tuple and a second loop for the same
predicate. _HERMES_PROTECTED_SUBPATHS already expresses "HERMES_HOME subpaths the
file tools must not rewrite", so the two secret directories join it and the
duplicate loop goes. Tests trimmed to two invariants: every read-denied secret
store is also write-denied on both the profile and the global root, and the
#45947 control files plus same-named paths outside HERMES_HOME stay writable.
Narrow the fix to secret material and stop reverting #45947.
The earlier revision added every read-denied name to the write denylist,
which re-blocked auth.json and webhook_subscriptions.json and rewrote the
test guarding them. #45947 freed those control files deliberately:
containment belongs in Docker/remote backends and OS permissions, not an
expanding hardcoded denylist.
What #45947 kept blocked is secret material, and that list had drifted:
auth/google_oauth.json (OAuth token store), the plaintext Bitwarden cache,
vault/ (key + ciphertext side by side) and browser-profile/ (copied
cookies / Login Data) were writable via write_file / patch.
- Write-deny those four; control files stay writable and read-denied.
- _WRITE_DENIED_SECRET_DIRS is its own tuple rather than _READ_DENIED_DIRS,
so a future read-only convenience deny cannot silently become a write deny.
- tests/tools/test_write_deny.py is restored unchanged from main.
- New tests pin both halves: secrets denied, control files writable, and
write denies stay a subset of read denies.
Fixes#110464
Read guards already refuse auth.json, webhook HMAC secrets,
google_oauth.json, the Bitwarden plaintext cache, vault/, and
browser-profile/. The write denylist only covered .env, the Anthropic
PKCE store, and bws_cache.enc.json, so write_file/patch could replace
the rest.
Drive the write denylist from the same credential names and
read-denied directories. Keep the extra write-only entries
(bws_cache.enc.json, state.db, sessions, pairing).
Fixes#110464
Related: #108716
The reply-pill fix split the body on a leading "> " so the strip would not
rewrite the "> <@bot:srv>" pill. That keyed the exemption on the body shape,
not on the relation: a hand-typed blockquote in a plain (non-reply) message
that mentions the bot inside the quote reached the agent with the raw
"@hermes:example.org" text, which main used to strip. Split around the quote
only when m.relates_to carries m.in_reply_to; otherwise strip the whole body.
Review finding: quote-block exemption keyed on body.startswith('> ') instead of m.in_reply_to.
Keep the two invariants that were red on main: a reply to the bot under
require_mention keeps reply_to_author_id, and a typed @bot in the reply text
is still stripped. Drop the plain-mention regression (covered by
test_matrix_mention.py) and the _split_reply_fallback table (change-detector).
The Matrix adapter strips the bot's mention from the inbound body before
_extract_reply_context parses the inline reply fallback, and that strip is a
blind whole-body replace. A reply to the bot names the bot in the fallback
pill ("> <@bot:server> quoted"), which is exactly what makes the message
count as a mention under the default MATRIX_REQUIRE_MENTION=true -- so the
strip runs on every reply-to-the-bot and rewrites the pill to "> <>", after
which the pill regex no longer matches. reply_to_author_id is lost,
reply_to_text becomes the mangled "<> quoted" remnant, and the prompt
renders "[Replying to: "<> ..."]" instead of "[Replying to your previous
message: ...]".
Split the body into (quote block, reply text) and strip the mention from the
reply text only. The mention gate still sees the raw body, so a reply to the
bot keeps waking the bot; the visible reply text and the quote-block strip
are unchanged.
Fixes#111233
NVIDIA NIM's Rust gateway 400s on list-type tool message content with a serde
error that names the enum rather than the field: "data did not match any
variant of untagged enum ChatCompletionRequestToolMessageContent". None of the
existing _MULTIMODAL_TOOL_CONTENT_PATTERNS match that wording, so the error
classifies as format_error (non-retryable) and the existing recovery —
downgrade the image-bearing tool message to text, remember (provider, model),
retry once — never fires: every retry re-sends the same shape and fails
identically in a session bound to that model.
Add the serialized enum name to the pattern tuple so the NVIDIA wording routes
to multimodal_tool_content_unsupported like the MiMo/Alibaba/Console Go
wordings already do (#111231).
is_browser_debug_ready now dials loopback /json/version through a
ProxyHandler({}) opener so a system proxy cannot capture the probe
(#110565); the TUI-gateway browser.manage tests stubbed urlopen, which
that path no longer calls, so every connect returned False.
The websockets dials got proxy=None but the three HTTP /json/version dials
(CDP override discovery, is_browser_debug_ready used by Lightpanda and the
real-profile readiness check, and surviving-Chrome detection) still resolved
via getproxies(), so under a system/env proxy discovery fell back to the raw
http:// URL, readiness never fired and /browser connect reported not ready.
loopback_request_kwargs() sits next to loopback_connect_kwargs() and is used
at all three sites (ProxyHandler({}) opener for the urllib one).
add_loopback_no_proxy turned an operator NO_PROXY=* into '*,127.0.0.1,...',
which urllib/requests no longer treat as the wildcard, flipping bypass-all
configs into proxy-all. A wildcard in either casing now leaves env untouched.
is_loopback_host also accepts any loopback IP literal (127.x, ::ffff:127.0.0.1).
Review finding: HTTP /json/version dials still proxied loopback; NO_PROXY=* wildcard broken by append.
Move the loopback NO_PROXY merge from browser_tool into agent/proxy_bypass.py (the
module that already owns NO_PROXY semantics) and reuse no_proxy_entries() so comma-
and whitespace-separated operator values are both preserved. Add
loopback_connect_kwargs() and pass proxy=None on the two in-process websockets
dials to loopback CDP endpoints (browser_cdp_tool._cdp_call, BrowserSupervisor._run):
those never see the child env, so the env merge alone left them routed through a
macOS system proxy. Remote CDP URLs keep the default proxy behaviour.
Tests trimmed to two invariants: the built child env appends loopback to an
operator NO_PROXY in both casings, and only loopback URLs get proxy=None.
Sibling helper in tools/browser_use_cli (#110570) is redundant once the shared
env carries the entries.
websockets>=14 defaults to proxy=True and resolves proxies via
urllib.request.getproxies(), which reads the macOS/Windows system
proxy config even with no *_proxy env vars set. Local CDP endpoints
(ws://127.0.0.1:<port>/devtools/...) were therefore dialed through
the system proxy and the handshake failed with "did not receive a
valid HTTP response" (#110565).
Append 127.0.0.1/localhost/::1 to NO_PROXY/no_proxy (both casings)
in _build_browser_env so every browser subprocess (Browser Use CLI,
agent-browser, Chromium, Lightpanda) bypasses proxies for loopback.
Operator-provided NO_PROXY entries are preserved.
Fixes#110565
Kanban cards have no length limit, but the session title store rejects
titles past SessionDB.MAX_TITLE_LENGTH with ValueError. _persist_session_title
reads that as a unique-title collision, retries with a "#N" suffix (longer
still), and the caller suppresses the second failure - so a worker spawned on
a >100-char card ended up with no title at all, where main at least gave it a
derived one. Trim the card title (with room for the "#N" retry suffix) before
persisting; a retried card now gets "<trimmed> #2" within the cap.
Review finding: >100-char card title left the kanban worker session untitled.
Follow-up to the salvaged commit from #111169 (@KoNit-K):
- Drop HERMES_KANBAN_TASK_TITLE. The worker already has HERMES_KANBAN_TASK
and HERMES_KANBAN_BOARD/HERMES_KANBAN_DB pinned in its env, so
maybe_auto_title reads the card title from the board itself (no new
HERMES_* env var for non-secret config; the dispatcher and the
delegation scrub list stay untouched).
- Unreadable or missing card: the session is named `Kanban task <id>`
with zero auxiliary calls (the fallback the issue asked for; the
#109743 seed left such workers untitled).
- The card title persists at `llm` authority via set_auto_title, so a
manual /title still wins and the upgrade thread never starts.
- Tests trimmed to two invariants against a real board + SessionDB
(card title, unreadable-card fallback), both red on origin/main.
The future-instant guard in claim_job_for_fire dropped the occurrence
identity for ANY claim ahead of the stored next_run_at. A hosted/webhook
fire for the armed slot that arrives a few seconds early (the fire
scheduler's clock runs ahead of ours) was therefore treated as an
off-tick run: it ran occurrence-free, mark_job_run recomputed the same
cron slot from a now still before it, and the tick/misfire backstop then
ran the slot a second time.
Only claims at least FIRE_CLAIM_SKEW_SECONDS (60 s) ahead of the slot are
now classified off-tick, so dashboard/manual far-future fires stay
occurrence-free while a skewed early fire keeps the slot identity.
completed_occurrence honours the same window so the early run's
completion row (finished just before the slot) still proves the slot
done instead of being discarded as poison.
Review finding: claim_job_for_fire future-instant guard had no skew tolerance; an early hosted fire for the armed slot ran twice.
tests/cron/test_manual_fire_occurrence.py (from #106970) overlapped the two smaller
invariants salvaged from #110419: an unclassified off-tick claim stays occurrence-free
(test_claim_job_for_fire.py) and a completion recorded before its occurrence is not
proof (test_scheduled_occurrence.py). On-time / late binding is already pinned by
test_scheduled_occurrence.py::test_ledger_migration_and_completion_identity.
Direction (ii) of the t_a662ca7a fix menu, ported from the live install
(ffffab1bd6 on the deployed checkout) and layered on top of the already-
landed direction (i) (ac10770894, manual= flag): claim_job_for_fire() now
also declines to bind an occurrence whose instant is still in the future.
A scheduled tick only fires when now >= next_run_at (_evaluate_due_job
returns False while the stored occurrence is in the future), so a claim
arriving BEFORE the stored next occurrence cannot be the tick that owns
it: it is a manual / dashboard / webhook fire (including the
claim_ttl_seconds reclaim after an expired lease, which arrives with no
manual flag) and stays occurrence-free - ledger row scheduled_instant=NULL,
next_run_at untouched. On-time and late (catch-up) ticks keep the exact
at-most-once occurrence identity.
Live case 2026-09-08: job d88fde170fb4 [bot:carl] Daily Plan Evening
(0 19 * * *), manual run at 19:53 stamped scheduled_instant=
2026-09-10T02:00:00+00:00 (= next day 19:00 PDT) status='completed';
completed_occurrence() then refused every later manual fire ("Job is
already being fired by the scheduler; not run again.") and the next
scheduled tick dedupe-skipped its real delivery. Pre-fix poison rows in
existing ledgers are not repaired by this commit; repair remains
audit-preserving UPDATE-only via the retired reference script.
Regression test (tests/cron/test_manual_fire_occurrence.py): two
invariant tests proven red on base b88e6776f4 (mid-cycle manual fire and
webhook claim_fire bind the future instant) plus three green-on-base
guard nets (on-time tick binding, late catch-up binding, forced-fire
occurrence-free). Red on base: 2 failed / 3 passed; green post-fix:
5 passed. tests/cron suite: 1232 passed, 1 skipped, 1 pre-existing
environmental failure in
test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code
that fails identically on base.
Kanban: t_d36f3b54 / t_2a050d21 (defect t_a662ca7a)
A claim that expired without a worker ever spawning (worker_pid NULL) was
reclaimed and immediately re-claimed on every dispatcher tick, with
consecutive_failures stuck at 0 — nothing could trip the breaker. Route the
reclaim through _record_task_failure (own txn after the reclaim commit, same
shape as enforce_max_runtime) instead of the salvaged raw counter increment,
so per-task max_retries / kanban.failure_limit and the gave_up event apply
and last_failure_error carries the stale lock. reclaim_task (operator path)
still resets the counter; the live-worker extend path never reaches it.
Trims the salvaged tests to one invariant that walks the breaker to its trip.
release_stale_claims() reclaimed expired claims without counting the
failure, so a claim that was never spawned (worker_pid NULL) spun forever
with consecutive_failures stuck at 0 and the failure_threshold breaker
could never trip (#111306). The reclaim transaction now increments
consecutive_failures atomically. Extend-live-worker and manual operator
reclaim paths are untouched (extend is not a failure; reclaim_task
deliberately resets the counter).
Tests: 3 new regression tests in tests/hermes_cli/test_kanban_db.py —
reclaim without spawn increments (red on base), reclaim with dead worker
increments (red on base), live-worker extend does not. Related kanban
suites: 79 passed, 1 skipped.
authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction
The budget in _sweep_agent_cache_under_pressure comes from the gateway's own
cgroup memory.high/memory.max, but read_anon_rss_mb() read /proc/self/status
RssAnon: only the main process. Every child in the unit (execute_code kernels,
terminal commands) is charged against the same limit, so a kernel at 4.7 GiB
pushed the unit to MemoryHigh while the valve saw <1.6 GiB and never fired;
systemd's stop then SIGKILLed the gateway mid-flush (the #80764 signature).
Under a capped cgroup v2, read own memory.stat `anon` (the same scope as the
budget); uncapped, or when the file is unreadable, keep the self reading.
_finite_limit is factored out of _cgroup_limit_bytes so the cap check and
the budget agree on what "unlimited" means.
Fixes#110549
The mid-turn /steer marker is delivered as a standalone role:"user"
message right after the newest tool result (steer_user_row /
apply_pending_steer_to_tool_results). But STEER_CHANNEL_NOTE (the
model-facing briefing) and the module comment still said Hermes
'appends their message to the end of a tool result'.
That stale claim briefs the model to expect the marker INSIDE tool
output, so a real standalone user row carrying the marker can read as
off-channel — the under-trust half of #110979. Correct the briefing and
comment to describe actual delivery; add a contract test tying the note
wording to the steer_user_row delivery mechanism (proven red on the old
wording).
#108319 moved hooks.outbound[].secret_env from os.environ to get_secret, which fails closed
outside a profile scope while multiplexing is on. The launch profile's hooks: block is
registered from start(), before any turn scope exists, so the first secret_env target raised
UnscopedSecretError, register_from_config propagated it, and _register_config_hooks swallowed
it at DEBUG — every launch-profile outbound webhook (signed or not) was silently dropped on a
multiplexed gateway. Secondaries were unaffected: they register inside their own scope.
The launch profile now gets the same treatment: _register_launch_profile_config_hooks enters
_profile_runtime_scope(get_process_hermes_home()) when multiplexing is active, then registers.
Not the environ fallback — a scopeless multi-profile read has no authority over the launch
env; the spawn site is where the scope belongs. The swallow logs at WARNING: a dropped hook
block is not debug noise.
Two invariant tests: multiplex on, no scope, secret in the launch .env -> both targets
register, the signed one with the launch secret (0 register on main); a registration failure
surfaces at WARNING.
#108319 made the release thread carry the caller's context and enter the owner's profile
scope only when no scope was present. But the LRU-cap eviction runs inside the REQUESTING
turn (run_turn_runner: _enforce_agent_cache_cap / _release_evicted_agent), and the LRU
agent it evicts may belong to another profile — so with profile A's scope active, profile
B's end-of-session memory commit ran under A's scope and A's home: B's transcript extracted
into A's provider namespace with A's credentials. Before #108319 the same commit ran under
the launch profile; the fix moved the leak, it did not close it.
_run_release_in_profile_scope now resolves the owner from the session key first — a named
profile's home, else the DEFAULT profile at the root Hermes dir (agent:main: keys), which
is not the launch profile when the gateway runs under `hermes -p x` — and enters that
profile's scope whenever the current one is not already the owner's (compared via
hermes_home_key). Same-profile in-turn evictions and the unscoped housekeeping sweep behave
as before. A failed owner lookup is logged at WARNING instead of silently swallowed, since
it reassigns the commit to the default profile.
Three invariant tests: A's turn evicting B's agent commits under B; a secondary's turn
evicting a default-profile agent commits under the default home; the same with the gateway
launched under a named profile still commits under the root. All red on main.
The share-code codec encodes createdBy through a fixed table that only
knew none/agent/user, so idxOf returned 0 for 'learn' and a learned
skill round-tripped as createdBy: null. The 2-bit field has a free slot,
so 'learn' now occupies it, and metaBadges labels agent- and
learn-created skills alike as 'learned' — matching the Python side,
where both values count as learning signals.
Review finding: learn nodes lost createdBy in share-code round-trip.
Replace the predicate unit test with an invariant on the real builder: a skill recorded
by a foreground create is in build_learning_graph() with zero uses, and an unmarked
never-used local skill is not. `hermes curator list-unmanaged` prints the actual
marker (created_by:learn) instead of hard-coding created_by:null. Docs: curator.md and
memory.md describe the learn marker and what the journey shows.
teknium1