Port from Kilo-Org/kilocode#13224: fixed waits belong in foreground terminal calls, while background mode is reserved for independently running processes.
When a child's final answer still missed its output_schema after the one
bounded retry, the result entry flipped to status=failed with the error
"Final answer does not satisfy the declared output_schema" — the completion
line printed ✗ and orchestrators read a finished audit as a failure. Five
audits of 413-4103 s were lost this way in the Sep 10-14 retrospective and
the parent had to mine the live transcripts; in four of them the "violation"
was a ```json fence around a valid array, which the candidate extractor
sliced to its first..last object.
Now: status stays completed, `summary` is the child's raw final text,
`schema_valid: false` + `schema_errors` carry the verdict and a `schema_note`
says the text is unvalidated; the sync completion line shows ⚠ with the
reason. The extractor tries the earliest-opening bracket span and keeps the
first that parses (fenced arrays validate). The OUTPUT CONTRACT the child
sees now says "ONLY the JSON value — no prose, no code fence" and what a miss
costs. One bounded retry is unchanged.
HERMES_DELEGATED_CHILD_CONTEXT=1 is deliberately carried into every shell/
execute_code subprocess a delegate_task child spawns (the fence must survive
exec so a grandchild `hermes kanban complete` cannot promote itself). But the
readers treated the bare flag as "fence every Kanban DB": kanban_db_connect
opened ANY board ?mode=ro and write_txn refused ANY mutation. A subagent
running a Kanban reproduction against a scratch HERMES_HOME therefore got a
silently read-only board with a misleading "descendants require an
initialized board" error; only one lane in the retrospective ever discovered
why (deleg_15dac332), every earlier kanban repro ran degraded.
The marker's value is now the fenced board ROOT (kanban_home() at spawn) and
readers deny only paths under that root or the dispatcher-pinned
HERMES_KANBAN_DB (kanban_path_is_fenced). In-process children and a legacy
"1" marker still fence everything; an inherited path marker is never
re-derived, so a grandchild that moved HERMES_HOME cannot unfence the real
board. Owner-gate tests (test_kanban_descendant_scope, cron env isolation,
kanban CLI exit status) are unchanged and green.
A delegated child's execute_code kernel was keyed correctly
(<owner>::child::<session>) but counted against the process-wide
max_session_kernels LRU cap (default 4) like any other kernel. In a fan-out
wider than the cap every child's first cell spawned a kernel and evicted the
oldest sibling's, so the sibling's next cell started a fresh interpreter and
NameError'd on state its own previous cell had set — while the tool schema
promised "variables, imports, and loaded data survive across execute_code
calls". Finished children's kernels also squatted the cap for
kernel_idle_timeout (1800 s) after the child was gone. 48 NameErrors across 28
subagent lanes in the Sep 10-14 retrospective.
A live child's kernel (local and remote) is now pinned: exempt from LRU
eviction while the child runs, disposed by the delegation cleanup path
(shutdown_kernels_for_delegated_child) as soon as the child finishes. Top-level
sessions keep the existing cap and idle reaping unchanged.
_handle_session_expired_and_retry only reached the at-most-once guard when a
reconnectable server record existed; without one (server torn down, MCP loop
not running) a write-capable call fell through to the generic "MCP call
failed" error, which invites the model to replay a write that may already
have landed. The session-expired classification now runs first and a
write-capable call always gets the outcome_uncertain error; the reconnect is
attempted only when a server can be signalled.
_track_inflight_rpc's teardown RuntimeError said "retry the request on the
rebuilt session" for every op; for a write-capable tools/call it now says the
request may already have been dispatched and must be verified first, so the
wording matches the at-most-once contract the recoverer enforces.
Docs: the readOnlyHint row explains that the same hint gates auto-retry after
a mid-call session expiry, and that unannotated tools on an idle-TTL
Streamable-HTTP server return outcome_uncertain on the first call after idle
instead of being transparently replayed.
A 'session expired' / transport-closed failure can arrive AFTER the server
already accepted and executed the request (proxy-synthesized 404s, pod
rotation, ClosedResourceError firing mid-response). Auto-retrying a
write-capable tool in that window risks a duplicate side effect that MCP
offers no way to undo.
The session-expired recovery path now consults the discovery-time
readOnlyHint capture (same data the trust gate uses): only tools whose
annotation is exactly True keep the reconnect+retry-once behavior. Write-
capable calls still get the transport healed (reconnect, breaker reset on
success) but return a structured outcome_unknown error telling the model
to verify with a read before re-invoking.
The OAuth 401 path keeps its retry for all tools: a 401 means the server
demanded authorization before dispatch, so the call never executed —
matching the upstream classifier's McpAuthRequiredError => safe rule.
Fails safe: missing/unknown annotations classify as write-capable.
The PR added four test functions for one fix. The host-side short-write and
multibyte cases are the same invariant as the sandbox size probe (an archive
that is not byte-exact is never referenced to the model), so they become two
parametrized rows of test_size_probe_decides_lossless, which now also uses
multibyte content so every row pins the byte-vs-char comparison. Net new
tests for the fix: 2 functions.
Also names in _write_to_sandbox why the +1 tolerance is keyed on heredoc
mode only: the payload backend delivers stdin verbatim and is expected to be
byte-exact. The three bare write_text() calls the footgun scanner flags in
this file gain encoding= while it is being touched.
Five near-identical MagicMock scripts pinned the same contract (byte-exact
or discard, heredoc gets exactly one extra byte); one parametrized test
plus the unprobeable-backend case cover it. Drop the host-side happy-path
test that duplicated test_multibyte_content_verified_by_byte_count.
Docstring now names the real heredoc wrapper
(BaseEnvironment._embed_stdin_heredoc) and notes the extra exec RTT per
oversized result (review point).
Oversized tool results are archived to disk and replaced in-context with a
'Full output saved to: <path>' reference. Until now the write was trusted
blind: a partially-flushed host file (ENOSPC/quota races) or a lossy sandbox
write (API-body truncation on payload backends) still produced the archive
reference, so the model was told the full result was recoverable when bytes
had silently vanished.
Both persistence paths now round-trip-verify size before building the
reference and fail closed to the bounded inline truncation otherwise:
- _write_to_spillover: byte-count check via os.stat after write; mismatched
archives are deleted and the caller falls through to inline truncation.
- _write_to_sandbox: wc -c probe after the cat; heredoc-mode backends get a
+1 byte tolerance (wrap_modal_stdin_heredoc appends one newline by
construction), unprobeable backends stay best-effort success.
Regression tests fail without the fix (verified by stashing the source
change: 4 failed). E2E-verified against a temp HERMES_HOME with real file
I/O including multibyte content and a simulated short write.
fal's LTX 2.5 fast endpoints accept 6-20s only up to 1080p — "At 1440p and
2160p, all frame rates support up to 10 seconds" — so a 4K request with the
family's 20s ceiling was rejected by the vendor. Families can now declare
`duration_cap_by_resolution`, applied after the enum snap / range clamp on the
resolved resolution enum.
An unset duration on a duration_enum family also snapped to enum[0] (6s),
silently overriding the endpoint's own "auto" default; None now omits the key
for enum families exactly as it already did for range families.
test_managed_media_gateways asserts the alibaba/happy-horse/ namespace by
prefix rather than the exact v1.1 literal so the next version bump doesn't
flip an unrelated gateway test.
`durations` carried two meanings told apart only by len==2 and gap>1: a
(min, max) range to clamp, or an enum to snap. A family with exactly two
legal values would have been misread as a range (review finding on #91311).
`durations` is now always the (min, max) window (what capabilities()/
list_models() read) and families with discrete values add `duration_enum`;
_clamp_duration takes the family and branches on the key, not the shape.
Also: restore the exact v1.1 endpoint assertion in the gateway namespace
test (a startswith/endswith check would not catch a silent version drift),
add the ltx-2.5 i2v snap case, and keep happy-horse on audio_native (the
schema test forbids audio+audio_native together, and v1.1 audio is always on).
The managed-gateway test asserted the literal v1.0 endpoint ids; its
stated purpose is verifying the alibaba/ (not fal-ai/) namespace. Assert
prefix+modality-suffix instead so version bumps don't break it.
test_silent_stall_still_times_out kept the 0.1s idle window and flaked
once locally under heavy load (load avg ~250): the child was killed
before its single stderr line was read, so the pre-stall-output
assertion saw an empty stderr. Same class as the progress test this PR
de-flakes; give it the same 0.25s budget. The 30s sleep still trips the
window, so the must-time-out direction is unchanged.
andrexibiza's review is right: the base test already printed tick 0
before its first sleep, so this change never removed a
sleep-before-first-tick race. The actual de-flake is the larger idle
budget (0.1s -> 0.25s) plus a longer heartbeat sequence whose ~400ms
runtime still exceeds the idle window. Say exactly that so the causal
record is accurate.
The stderr-progress idle-timeout test used a 0.1s idle window with 0.04s
ticks — shorter than Windows process spawn, so the first chunk could never
arrive in time (deterministic failure on Windows, flake under Linux CI
load). Verified failing identically on pristine main before the change.
Fix: emit the first tick immediately, tick every 50ms for ~400ms total,
250ms idle window (5x tick period). The pass still depends on the progress
extension while tolerating real spawn/scheduling latency.
Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com>
mcp 2.x rejects an authorization response that omits the RFC 9207 `iss`
parameter when the authorization server advertised
`authorization_response_iss_parameter_supported`. Cloudflare advertises it
AND sends it; the CLI loopback handler has always forwarded it, but every
other callback producer parsed only code/state/error, so the SDK raised:
OAuthFlowError: Authorization response missing iss parameter
advertised by the authorization server
and the server parked. Same machine, same config, `hermes mcp login <name>`
from a terminal succeeded — the failure is specific to the non-CLI relays.
Forward `iss` on every producer, matching `_make_callback_handler()`:
- tools/mcp_dashboard_oauth.py: `deliver_callback()` accepts `iss`;
`wait_for_callback()` returns `(code, state, iss)`. The bridge in
tools/mcp_oauth.py already splats that tuple into
`_authorization_code_result(code, state, iss)`, so it needs no change.
- tui_gateway/mcp_oauth_sessions.py: the gateway-hosted loopback listener
parses `iss`, and `deliver_callback_flow()` forwards it.
- tui_gateway/methods_tools.py: the `oauth.callback` RPC passes `iss`.
- hermes_cli/web_routers/mcp.py: the dashboard callback route accepts it.
- apps/desktop/electron/mcp-oauth-callback-ipc.ts: the one-shot listener
reads `iss` off the redirect (the renderer already spreads the whole
callback object into the RPC, so it flows through unchanged).
Providers that omit `iss` round-trip as `None`/`null` rather than being
dropped, so servers that do not advertise RFC 9207 keep working.
Verified live on Windows against mcp.cloudflare.com, whose metadata sets
`authorization_response_iss_parameter_supported: true`: the server that
previously parked on the missing-iss error now reports
`Authenticated — 3452 tool(s) available` and `hermes mcp test cloudflare`
connects. State-mismatch and replay rejection are unchanged.
Tests (each fails on base, passes with the fix):
- test_dashboard_flow_preserves_rfc9207_iss
- test_deliver_callback_forwards_iss (client-redirect relay)
- test_loopback_listener_forwards_iss (real HTTP redirect)
- two vitest cases on the Electron listener, incl. the iss-absent case
Refs #92758, #99984. PR #92765 fixes the dashboard route and the loopback
listener but not the client-redirect relay
(`deliver_callback_flow` / `oauth.callback` / the Electron listener), which
is the path Desktop drives against a remote backend.
The 400-recovery reload installs a disk pair and only then re-runs issuer
binding on it. When that pair was minted by a different issuer the enforcer
strips its refresh token, and the reload must report "no recovery" so the
session is cleared like any other dead grant. No test pinned that verdict:
a reload that ignored the install result would keep a stripped pair in the
context and return True. The new invariant drives a real 400 through
_handle_refresh_response against a foreign-bound disk pair and asserts the
result is False, the context is cleared, and the foreign refresh token does
not survive on disk.
_hermes_live_ttl read expires_in off current_tokens without checking for
None; getattr(None, ...) happened to yield the default and report "live".
Both current callers install a pair first, but the helper is now explicit
that an empty context is never live, so a future call site cannot adopt
nothing.
Two defects in the peer-adoption path of the refresh fence:
1. `_hermes_install_disk_pair` raised `_RefreshCompletedByPeer` when issuer
binding stripped the candidate's refresh token. That is the right outcome
for `_refresh_token` (restart the flow so the SDK lands in 401 -> full
auth), but `_hermes_reload_tokens_after_refresh_failure` shares the helper
and must instead treat the candidate as rejected: restore the previous
pair and return False so the caller clears state and prompts. The helper
now returns whether a refresh token survived binding and each caller
decides; the one-shot adopt wrapper is inlined into `_refresh_token`.
2. `_hermes_live_ttl` treated `expires_in is None` as expired. RFC 6749 makes
`expires_in` optional, the SDK's `is_token_valid()` is True with no expiry,
and `_rebase_expires_in` preserves None on read, so a peer's rotated pair
without an expiry was never adopted and we POSTed its refresh token
anyway, burning a generation on single-use providers. None now counts as
live; the try/except around a pydantic `int | None` field is dropped.
One new test drives the real auth flow against a peer pair with no
`expires_in` and asserts the pair is adopted with zero POSTs.
The SDK drives a refresh as a generator (request yielded from
_refresh_token, response consumed in _handle_refresh_response), so the
fence was a hand-driven @asynccontextmanager: __aenter__ in one method,
__aexit__ in another, generator object stashed on the provider. A plain
`acquire_refresh_fence(path, timeout) -> fd` / `release_refresh_fence(fd)`
pair says what actually happens and leaves nothing half-entered to leak.
The descriptor is opened with os.open at 0600 and closed on every
acquisition failure.
The `_refresh_token` release-on-exception stays: `_refresh_token` is also
reachable outside `async_auth_flow` (tests call it directly), and the
wrapper's finally only covers the generator-driven path.
HermesTokenStorage.remove() now unlinks the `.refresh.lock` sibling so
logout leaves no stray file. It is deliberately NOT added to
_state_paths(): snapshot()/restore(only_if_absent=True) treat any
existing state path as "newer state exists", and a lingering lock file
would silently veto a rollback.
tests/tools/test_mcp_oauth.py: drop the unused `Path` import (`time` is
still used by the socket poll helper).
The poll loop swallowed every OSError from the lock syscall as contention,
so a filesystem that cannot take advisory locks at all (ENOLCK on some
network mounts, EMFILE, ...) stalled for the full 60 s deadline and then
blamed a peer. Only EWOULDBLOCK/EAGAIN/EACCES/EDEADLK mean "held by
someone else"; anything else now raises RefreshFenceTimeout immediately
with the real errno. Still fails closed -- the refresh is never POSTed
without ownership -- but the failure is diagnosable and instant.
The errno set mirrors cron.scheduler._is_lock_contention_errno; it is
duplicated rather than imported because importing the scheduler pulls in
the whole cron module graph for a four-value tuple.
Both fence paths that pull a peer's pair off disk now go through
_hermes_rotated_candidate (different, non-empty refresh token + non-empty
access token) and _hermes_install_disk_pair, which runs
enforce_refresh_token_issuer on the installed pair. Before, the adopt path
skipped the issuer check entirely, so a pair minted by a different issuer
could be POSTed straight to the new one.
The adopt path installs the candidate even when its access token has
already expired: the POST we are about to build needs the new refresh
token, and skipping the POST (_RefreshCompletedByPeer) is only correct
when the peer's access token is live with a positive TTL, mirroring the
reload path's clamp-to-zero guard. When the issuer enforcer strips the
refresh token there is nothing to refresh with, so the flow restarts into
401 -> full authorization instead of failing with OAuthTokenError.
The reload helper drops its outer except-Exception: get_tokens already
returns None for absent or corrupt files, so the blanket catch only hid
programming errors. Comment updated: this path exists for writers outside
the fence (interactive login, pre-fence Hermes), not for a fenced peer.
`_token_store_lock` serialized a single get_tokens()/set_tokens() call and
then released. Its two justifications no longer hold:
- torn reads: `_write_json` goes through `atomic_json_write` (write to a
sibling, rename), so a reader can never observe a half-written token
file, locked or not;
- the read-modify-write of a single-use refresh token: a lock released
between the read and the POST cannot close that race. `_refresh_fence`
now spans read -> POST -> persist, and every store access on the refresh
path (adopt-from-disk read, post-failure reload, `_store_tokens` write)
runs inside it.
The remaining unfenced accesses are the cold `_initialize` read and the
authorization-code exchange's full overwrite -- neither is a
read-modify-write, so neither needs mutual exclusion. Keeping a second,
fail-open lock layer only adds a 10 s stall on a stale lock file with no
correctness gain. Tests that exercised the removed lock go with it.
The 433-line harness spawned three interpreters and an HTTPServer to show
that one refresh generation is consumed once. The same invariant holds
in-process: flock is per open file description, so two real provider
instances sharing one token store contend for the fence exactly like two
processes do, and the SDK auth flow can be pumped with asend() the way
httpx does. Two tests now bind the fix deterministically:
- two providers, one store, a single-use token endpoint: exactly one POST
carries R1 and both providers end holding the rotated pair (the loser
adopts from disk and never presents the burned grant);
- fence held by another holder past the deadline: the refresh fails
closed, no POST is sent and neither memory nor disk loses the tokens.
The token-store lock is per-operation: get_tokens() and set_tokens() each
take it and release it. With a provider that issues single-use refresh
tokens, two processes can therefore both read R1, both POST it, and the
loser gets invalid_grant on a session that was healthy:
A: get_tokens() -> R1 (lock taken and RELEASED)
B: get_tokens() -> R1 (lock taken and RELEASED)
A: POST R1 -> 200, receives R2
B: POST R1 -> 400, credential already burned
Add _refresh_fence(), held across read -> POST -> persist so exactly one
process consumes a refresh generation. It fails CLOSED: unlike the
token-store lock it raises RefreshFenceTimeout instead of degrading to
unlocked, because proceeding without ownership is the race itself. It
locks a .refresh.lock sibling rather than the token file, since
flock/msvcrt locks are per-descriptor and nesting one path would
self-deadlock on Windows and silently no-op on POSIX.
The provider takes the fence before its final read, re-reads under it so
a peer rotation is adopted instead of overwritten, and releases in
_handle_refresh_response. async_auth_flow also releases on abandonment:
a cancelled generator never reaches the handler, which would strand the
fence and turn the race into a deadlock. That wrapper delegates send and
throw manually -- async generators have no yield from, and `async for`
would feed the SDK response to the inner generator as None.
tests/tools/test_mcp_oauth_refresh_fence.py is an acceptance test, not a
unit test: two real OS processes refresh against a single-use-token
authorization server that audits every redemption. It asserts R1 is
presented exactly once, neither process clears the session, and disk
converges on the newest token. Verified to FAIL without the fence
(audit=[rt-1, rt-1]); a threading-only lock cannot catch this.
(cherry picked from commit c1508a1d47e2cbd94e05fa507684f3716a1e988c)
The desktop app spawns 'serve' while the scheduled task runs 'gateway run',
so two backends routinely share one HERMES_HOME. With a provider that issues
single-use refresh tokens, both could POST the same token and the loser's
refresh was rejected, clearing an otherwise healthy session.
Guard the token file's read-modify-write with a bounded advisory file lock
(fcntl on Unix, msvcrt on Windows, in-process only where neither exists),
mirroring how cron/jobs.py guards jobs.json. Acquisition is non-blocking with
a 10s ceiling: a briefly-contended refresh beats a permanently stuck client.
(cherry picked from commit 16f0d811de446a66ed5fd061fd7594fca3d230da)
`\bsocat\b` under IGNORECASE matched "SOCAT", the Surface Ocean CO2 Atlas,
in every oceanography skill of a 2,110-file research bundle (17 critical
findings in one file), burying the bundle's real issues under noise. A real
socat relay always names an address type (TCP:/UDP:/OPENSSL:/EXEC:/SYSTEM:/
PTY:/UNIX-…:), so the pattern now requires one on the same line. `nc -l` /
`ncat -l` are unchanged. Scanner version bumped to v5 so cached verdicts
re-scan.
`ProcessRegistry._move_to_finished` pops the session out of `_running`, then
saves the receipt, releases handles and writes the checkpoint, and only THEN
enqueues the completion and sets `_completion_event`. A quiet one-shot parent
whose turn ends inside that window called `wait_for_pending_completions`,
found nothing in `_running`, drained an empty queue and exited without the
follow-up turn. That is the CI flake in
tests/tools/test_completed_process_results.py::test_headless_terminal_result_survives_cli_exit
(`follow_ups == []`), which also hit unrelated branches.
Consider `_finished` sessions whose event is not yet set as pending too.
Repro: a 6s sleep before the enqueue plus a 2s delay before the parent's first
wait fails the E2E 2/2 on main and passes 2/2 with this change.
Bot-to-bot message_agent delivery builds both transport argvs (local
teammate chat and peer dm) with a bare "hermes" as argv[0]. Since #96631
the delivery runner spawns under terminal_tool's isolated host-local
environment, which does not inherit the gateway's PATH — so on
docker/service installs (venv at /opt/hermes/.venv) every delivery exits
with FileNotFoundError: 'hermes'.
Resolve the CLI with bot_relay._hermes_cli() (#93590) — the venv sibling
of this interpreter, then shutil.which, then the bare name — at both
argv construction sites. The turn-lock matcher in _delivery_lock()
already matches argv[0] by basename, so absolute paths lock exactly as
before.
Fixes#100662
Masking every balanced [x](dest) in a .md file also blanked
`cp [k](../../../.ssh/id_rsa) /tmp` inside a ```sh fence, which main scored
caution and the branch let through as safe. A link inside a code fence is a
command argument, not a hyperlink: toggle masking off between fence markers.
Split the salvaged regression into the two facts the fix must hold:
a 3-level doc link no longer produces a traversal finding or blocks a
community install, and traversal outside a link destination (a shell
script, or prose on the same line as a link) still fires. The shell
script case is taken from #110978 by @KoNit-K; the same-line case is
what distinguishes masking link destinations from gating by extension.
Review of the write-verb gate found sed -i, chmod, truncate, curl -o, wget -O,
git clone and a scripted open(...) against ~/.ssh all slipping to no finding,
where the bare path regex on main caught them. Add those verbs and the open(
shape to the gate; read-only mentions stay clear.
Salvage trim of #89249:
- keep pattern id `ssh_access` (test_memory_tool and callers key on it; renaming buys nothing)
- word-bound the verb alternation: the unanchored form matched `add` inside "address" and
`dd` inside "middle", so read-only prose still fired; `\b` closes that leak
- fold the second "bare leading redirect" regex into the same alternation (`>>?` branch)
- tests: one parametrized "write shapes still fire" (echo, cat, cp, tee, mv, install,
printf, dd, scp, rsync, ln, leading redirect, option clusters) and one "read-only
mention does not fire"; the trade-off/change-detector tests are dropped
- test_memory_tool: the persistence fixture used a read-only mention; use a write shape
Extend the write-verb alternation with mv/install/printf/dd/scp/rsync/ln,
allow short option clusters between verb and path, and add a bare-leading-
redirect branch (a > ~/.ssh/... line carries no verb word at all). Prose
that names a write primitive before an SSH path still fails closed — a
false positive costs a review, a false negative is a backdoor.
(cherry picked from commit 4865b96ca3c8661c0ea6f5c479c198ccf68f86c4)
The bare \$HOME/.ssh|~/.ssh regex fired on ANY mention of SSH paths in
scanned content, so operational documentation (VPS recovery notes, SSH
configuration write-ups stored in memory or skills) was blocked as a
persistence threat. Require an echo/cat/cp/tee/append/add/write/>>
verb before the path so only backdoor-insertion shapes match.
(cherry picked from commit 8c9b4c8972ac849e4c82fa20d355733ca3e5a231)
Extend the two salvaged invariant tests instead of adding more: the
`.profile` negative now covers `?.`, `().` and `].` access (the
lookbehind was widened from `[A-Za-z0-9_]` to `[\w)\]?]` during salvage),
the positive covers a bare `.profile` token, and the sudo positive covers
the `-S` (read password from stdin) form, which the gateway's own masked
password prompt relies on.
`sudo.request` and `sudo.respond` are gateway wire events: the masked
sudo-password prompt the terminal tool raises, which every client surface
(desktop, TUI, any plugin that relays secure prompts to another device)
has to name to forward it. The `sudo_usage` rule matched the bare word,
so any plugin listing those events scored `high` and every install of it
landed on `caution`, which Hermes Desktop cannot confirm past.
A dotted event name is never a shell `sudo` invocation. Exclude exactly
those two names with a negative lookahead; a real `sudo cmd` still fires.
(cherry picked from commit a7a3a31126de8057c2dbb9b7048724aa9d55a7fd)
The shell-startup-file pattern is
`\.(bashrc|zshrc|profile|bash_profile|bash_login|zprofile|zlogin)\b`.
Six of those names are distinctive enough that seeing them after a dot
means the file. `profile` is not: it is also how every language spells
attribute access, so `self.profile`, `user.profile`, and
`request.profile` each score a medium persistence finding.
The cost is signal, not blocking -- `_determine_verdict` treats
medium/low alone as informational -- but a plugin that happens to name a
field `profile` buries the findings a reviewer has to read. A model-
provider plugin whose tests exercise a `profile` object contributed 36
of 38 findings in its scan report, all of them this pattern.
Split `profile` into its own entry anchored on a non-identifier
character before the dot. Real references keep matching in the forms they
actually take (`~/.profile`, `"$HOME/.profile"`, `./.profile`,
bare `.profile`); attribute reads no longer do. The other six names are
untouched. Both entries keep the `shell_rc_mod` id, and scan_file
deduplicates on (pattern_id, line), so a line holding both still yields
one finding.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 705d0bddc61a1c55be82956d343b229544350eae)
The file-safety salvage carried five tests and the execute_code salvage four;
fold them into the two behaviour contracts per fix: (a) the named-profile scope
exempts the root's direct files and the write lands with no prompt; a child env
sees the ACTIVE profile's home per turn; (b) negatives hold: a checkout's
.hermes/config.yaml stays gated fail-closed, a lookalike profiles/ tree exempts
nothing; a dedicated process with no override is untouched. Both red on base.
Also compress the _hermes_exempt_homes docstring (WHY only; cite #110630).
Under `gateway.multiplex_profiles: true` every gateway start logged a WARNING +
traceback from `tools.registry` for `_check_vault_available`. The browser vault
gate is a `no_cache_check_fn`, so `_check_fn_cached` short-circuited into
`_run_check_fn_uncached(fn)` without consulting the cache scope and the default
`unresolved_scope=False` hint reported an EXPECTED boot-time fail-closed read as
"scope was resolved" — the #100697 fix only reached the cached branch.
Derive the verdict from `current_secret_scope()` at the catch site instead of a
branch-derived hint: no scope installed → DEBUG without traceback (expected,
re-probes on the first scoped turn); scope installed but the read still failed
closed → WARNING + traceback (the probe dropped the scope on a bare thread/hop,
a spawn-site bug that must stay loud). Every uncached probe that reads a
credential is covered, vault included.
Fixes#110635. Credit @KoNit-K (#110638) for the report-side diagnosis; that PR
disabled the legacy-cloud probe for the vault gate only, which would have hidden
the vault from legacy Browser Use cloud users and left the class open for every
other uncached probe.
Under a multiplexed Desktop/Dashboard connection, one server process serves
multiple profiles, binding a context-local HERMES_HOME override per turn.
_build_child_env scrubbed the server process's os.environ - which carries the
machine-default HERMES_HOME - so skill scripts run via execute_code silently
read/wrote the wrong profile's directory.
Rewrite HERMES_HOME in the child env from the active override when one is
bound (the same per-turn rewrite apply_subprocess_home_env already does for
HOME). With no override (dedicated per-profile process) the inherited value
is left untouched.
Fixes#110303.
Under a named profile (``hermes -p <name>``, i.e. ``HERMES_HOME=<root>/profiles/<name>``)
the protected-instruction gate exempted only the profile dir, so the ROOT's DIRECT
files (LEDGER.md / MEMORY.md / USER.md / SOUL.md / AGENTS.md / DECISIONS.md ...) fell
through to the ``.hermes`` component rule and were treated as project-local
``<repo>/.hermes/config.yaml``. That gate always-asks and fails closed without a human
channel, so every write there was refused headless — it blocked #54 (LEDGER.md edit
could not land). Subdirectories (scripts/, cron/, data/) were unaffected, which is why
#55 sailed through and #54 did not.
``_hermes_exempt_homes()`` now returns the active profile home plus the Hermes ROOT
when that home really is a named profile (``named_profile_home`` validates: ``.hermes``
name, home markers, tombstone, or the resolved default root), so a coincidental
``profiles/`` directory never exempts its parent. ``_get_real_hermes_home()`` keeps its
per-profile semantics — the #107327 multiplex regression test pins that — and the
exemption consumes the new helper instead.
Refs: https://gitlab.yx.netease.com/agent-projects/platforms/agent-mentor/-/issues/60
Fold the cherry-picked mechanism into the main file's compact style: one
denylist-owner regex replaces the assignment-target + name regex pair and the
_in_denylist_construct helper; the comment-prefix table loses its dead .css
entry; the denylist demotion lands on high (a confirmable caution) instead of
medium so the finding still gates the install and shell_rc_mod, already
medium, leaves the set. The verb guard widens to stem-prefix matching and
copyfile/copy2/sendfile per the review thread on #92632, closing the
Path.read_text()/shutil.copyfile shape. Tests trimmed to two invariants: a
skill's own denylist is caution (force-overridable), a real write to
authorized_keys stays dangerous.
`_scan_file` matches every threat pattern against every line with no notion
of what the line is. The path-token patterns — `authorized_keys`, `~/.aws`,
`~/.ssh` — therefore cannot tell `cat ~/.ssh/authorized_keys` from a skill
that spells the path in order to REFUSE to read it. One `critical` becomes
`dangerous` in `_determine_verdict`, and on a community source that blocks the
install with no `--force`, so the skill that bothered to skip credential
files is the one that gets quarantined (#92478).
Demote, do not drop, following the precedent `allowed_tools_field` already
sets in this file: the finding keeps its file, line and matched text so an
auditor still sees the token; it just stops deciding the verdict alone.
Two contexts qualify, and only for the eight path-reference pattern ids:
- a whole-line comment, in a language that HAS comments, drops to `low`.
Markdown is deliberately excluded: `#` opens a heading there, and Markdown
prose is the prompt-injection surface itself.
- a line inside a construct NAMED as a denylist (`SKIP_PATTERNS`, `DENY_*`,
`EXCLUDE_*`), carrying no verb that could touch the path, drops to `medium`.
The issue also suggested demoting any quoted token on a verb-free line. That
is wider than it looks — a fragment in quotes can be interpolated into a
command a line later — so the name test is the primary rule and the verb test
only guards it, because the construct's name is attacker-chosen.
The denylist check is statement-aware rather than per-line: the reported
match sat on a continuation line of a multi-line regex whose name is four
lines up, which a per-line test reads as anonymous.
8 tests. Reverting the demotion fails the two behaviour tests and leaves the
six contract tests green; dropping the verb guard, the Markdown exclusion, or
the statement-awareness each fails exactly its own test.
(cherry picked from commit f50acd34f62375926bd5843125e106f7e7eb6ee8)
Fold the three PRs' overlapping tests into two invariants per behaviour:
- comment/changelog prose demoted to caution and confirmable; trailing comments,
runtime code and agent-facing docs still dangerous (#111193)
- Markdown plan/design prose demoted to caution; the same content in runtime
code still dangerous (#103364)
- skills_guard: context_exfil needs a transfer directive; rm -rf under temp
roots is not destructive_root_rm
The #111199 regression test is kept (behaviour is the same); its mechanism
(re-read the file per finding, cap every .md) was not carried since the
match-based cap already covers it without touching agent-facing docs.
'//' is not a CSS comment marker, so .css is dropped from the prefix table.
(cherry picked from commit 93d65bed0f558d74c6dc1b752823715834c56078)
Note: mechanism (all-docs cap + file re-read for # comments) superseded by the match-based comment/changelog cap; only the regression test is carried
A whole-line code comment or a changelog entry describing the threat a defense
rejects ("# a symlink could point at /etc/passwd, so ...") scored full severity,
driving the verdict to dangerous and making security-hardened community plugins
un-installable: --force does not override dangerous, so the only way through was
deleting the documentation of what was defended against. Whole-line comments and
CHANGELOG entries now cap one severity step lower (critical->high), keeping the
finding visible and the verdict at caution: blocked by default, --force
overridable. Trailing comments (executable code on the line), runtime code and
agent-facing docs keep full severity.
Fixes#111193
(cherry picked from commit 2a79f0a67e70eddcd387e759b1570d4feaee1e97)
Teknium