Vitest regression that builds `dist/entry.js` and checks two
structural invariants required for startup to not hang:
1. Zero `async "<path>"() { … }` keys inside any `__esm` definition.
esbuild only emits the `async` form when a module body contains
top-level await; the `__esm` helper at the top of the bundle
does not await nested inits, so any async wrapper participating
in a circular module graph would deadlock the boot
`await Promise.all([…])` in `src/entry.tsx`.
2. No `node_modules/ink/build/index.js` or
`node_modules/ink-text-input/build/index.js` modules. Their
absence is what makes invariant 1 hold today; if a future commit
re-introduces the `ink-text-input` re-export, this test catches
it before the bundle ships.
The test rebuilds the bundle on demand when the source is newer than
`dist/entry.js`, runs in <100ms with no TTY needed, and is hermetic
on a clean checkout.
Update the comment on the `alias` entry to mention the second reason
the source-inline is needed: keeping the upstream `ink` /
`ink-text-input` graph out of the bundle (which fixed the startup
deadlock in #31227). Code path is unchanged.
The dashboard TUI bundle hung at startup with only 141 bytes of ANSI
reset sequences and a blank screen forever. Root cause: esbuild's
lightweight `__esm` helper at the top of `dist/entry.js` does not
await nested async init, so a circular async cycle in the module
graph never resolves. The cycle came from re-exporting
``TextInput`/`UncontrolledTextInput`` from `'ink-text-input'` here —
that npm package depends on the upstream `ink` package, whose graph
loops back through React + our in-tree `@hermes/ink` ink fork. The
result: `init_entry_exports` was emitted as `async … await
init_build4()` (where `build4` is `node_modules/ink-text-input/build`),
and the top-level `await Promise.all([init_entry_exports().then(...)])`
in `src/entry.tsx` deadlocked waiting on the dangling Promise.
Nobody in `ui-tui/` actually imports `TextInput` from `@hermes/ink` —
the composer uses the in-tree `src/components/textInput.tsx` widget
instead. Drop the re-export from the source so the bundle no longer
inlines the upstream ink graph at all. Callers that legitimately want
the upstream widget can still import it from the dedicated
`@hermes/ink/text-input` subpath, which sits outside `entry-exports`
and so does not get inlined into consumers' bundles.
After the fix:
* `dist/entry.js` shrinks from 2.9MB → 2.4MB (~11.5k fewer bundled
lines) with zero `async __esm` wrappers remaining.
* `init_entry_exports` is now a synchronous `__esm` module.
* The bundle's top-level await chain resolves in ~30ms instead of
hanging.
Subprocesses spawned by the terminal tool, execute_code, Docker backend, and
the codex app-server could inherit Hermes-internal secrets that the name-based
`_HERMES_PROVIDER_ENV_BLOCKLIST` can't enumerate, because they're injected into
`os.environ` at runtime under dynamic names:
- `AUXILIARY_<TASK>_API_KEY` / `AUXILIARY_<TASK>_BASE_URL` — per-task side-LLM
credentials bridged from `config.yaml[auxiliary]` by gateway/run.py and cli.py
(vision, web_extract, approval, compression, plugin-registered tasks). Often
separate, higher-spend keys plus base URLs pointing at private endpoints.
- `GATEWAY_RELAY_*_SECRET` / `_KEY` / `_TOKEN` — relay-auth material provisioned
by gateway/relay.
Additionally, agent/transports/codex_app_server.py built its spawn env from a
raw `os.environ.copy()`, bypassing the centralized `hermes_subprocess_env()`
helper entirely — handing every codex subprocess the full Tier-1 secret set
(GH_TOKEN, gateway bot tokens, Modal/Daytona infra tokens, dashboard session
token) unfiltered. This is the #29157 sibling spawn-site gap; copilot_acp_client
already routes through the helper.
Fix — single chokepoint:
- Add `_is_hermes_internal_secret(key)` in tools/environments/local.py as the
single source of truth for the dynamic secret patterns. Matches
AUXILIARY_*_API_KEY / _BASE_URL and GATEWAY_RELAY_*_SECRET/_KEY/_TOKEN; leaves
non-secret AUXILIARY_*_PROVIDER/_MODEL and GATEWAY_RELAY routing hints visible.
- Wire the predicate into every spawn path unconditionally (ignores skill
env_passthrough opt-in AND inherit_credentials — a model-driving CLI never
needs these): `_sanitize_subprocess_env` (both loops), `_make_run_env`
(foreground), `hermes_subprocess_env` (Tier-1), and the Docker forward filter.
- Add the static GATEWAY_RELAY_* names to `_HERMES_PROVIDER_ENV_BLOCKLIST` so the
exact-match path catches them independently of the predicate.
- Add the GATEWAY_RELAY_ID/_SECRET/_DELIVERY_KEY triplet to `_ALWAYS_STRIP_KEYS`
(Tier-1) so it is stripped unconditionally on EVERY spawn surface — including
the codex/copilot `inherit_credentials=True` path that skips the Tier-2
blocklist. `_SECRET`/`_DELIVERY_KEY` are already predicate-matched; `_ID` has
no secret suffix, so enumerating it here is what closes its leak on the
inherit path (self-review W1).
- Defense in depth: env_passthrough.py `_is_hermes_provider_credential()` now
consults the same predicate, so a skill can't register these names as
passthrough and tunnel them into an execute_code / terminal child.
- Route codex_app_server through `hermes_subprocess_env(inherit_credentials=True)`
— strips Tier-1 + dynamic-internal secrets while provider creds (which codex
needs to authenticate) still flow.
Consolidates PRs #53715 (necoweb3 — the _is_hermes_internal_secret backbone +
Docker filter), #53503 (srojk34 — env_passthrough guard), and #55709 (srojk34 —
codex routing). Retires #52348 (claudlos): its copilot half is already on main,
and its codex half used the full-strip `_sanitize_subprocess_env` which would
break codex provider auth — the correct tier is `inherit_credentials=True`.
Tests: TestHermesInternalDynamicSecrets (terminal + predicate + passthrough
override), TestInternalDynamicSecrets (hermes_subprocess_env both tiers),
TestSpawnEnvSecretStripping (codex spawn env), plus env_passthrough
defense-in-depth cases.
Co-authored-by: necoweb3 <sswdarius@gmail.com>
Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
Co-authored-by: claudlos <claudlos@agentmail.to>
AIAgent.run_conversation() promises a dict with final_response, but 16
terminal-failure branches returned dicts that either omitted the key or
set it to None. Callers that index result['final_response'] directly
(run_agent.py chat() + the __main__ printer) turn a real provider/context
failure into an opaque KeyError instead of surfacing the actionable error.
Every offending branch already carried usable 'error' text, so this
mirrors that text into final_response for all 16 sites (8 that omitted the
key, 8 that returned None). Adds an AST regression test that fails if any
run_conversation() dict return omits final_response or sets it to a literal
None, and tightens the invalid-response test to assert final_response == error.
The network-error reconnect ladder (#55992) captured a stable self._app
local across its awaits and failed fast when the adapter was torn down
mid-sleep. The 409-conflict retry path had the identical unguarded
self._app.updater.start_polling() deref — a concurrent disconnect()
during its RETRY_DELAY sleep would raise the same 'NoneType' object has
no attribute 'updater' and, on a non-final retry, land in limbo. Apply
the same stable-local + fail-fast pattern so the existing except block
reschedules or escalates to fatal.
A delayed fatal-error notification from an adapter instance that has
already been replaced by a successful reconnect (a different adapter
object now owns the platform slot) was still processed: it overwrote
the platform's runtime status back to retrying/fatal and could
re-queue an already-healthy platform for reconnection.
Snapshot the current owner of the platform slot at the top of
_handle_adapter_fatal_error and bail out before any side effect when
it belongs to a different, already-installed adapter.
_handle_polling_network_error's chained retry never updated
self._polling_error_task, so the reentrancy guard shared with the
heartbeat loop and the pending-updates probe went stale mid-recovery,
letting more than one recovery attempt run concurrently against the
same adapter. Combined with a TOCTOU window in
_handle_adapter_fatal_error (the adapter was only removed from
self.adapters in a finally block after awaiting disconnect()), two
concurrent fatal notifications for the same adapter could both pass
the "still installed" check and call disconnect() twice, which is
where the reported "'NoneType' object has no attribute 'updater'"
originates once self._app is cleared by the first call.
- Reassign the chained retry task to self._polling_error_task so the
guard reflects an in-flight recovery.
- Capture self._app in a local variable across the stop/start_polling
sequence instead of re-reading self._app between awaits.
- Claim (pop) the adapter from self.adapters before awaiting
disconnect() in _handle_adapter_fatal_error, not after, closing the
TOCTOU window for a concurrent notification on the same adapter.
Path(raw).name reduces '..'/'.'/'' to themselves, so basename
extraction alone still let a Graph-provided display_name of '..' or
'../' escape the temp recording directory (tmp_dir / '..' resolves to
the parent). Reject the dot-only basenames explicitly and fall back to
the artifact id. Extends @outsourc-e's regression coverage with the
dot-only cases.
The MoA aggregator received the per-turn reference block merged into the most
recent `user` message. In an agentic tool loop that message is the original
task near the top of the context (everything after it is assistant/tool turns),
so injecting text that changes every iteration diverges the prompt prefix early.
The server's KV cache then cannot be reused and the entire conversation
re-prefills on every tool-loop step — full prefill each step, which dominates
latency on long contexts.
Append the reference block at the end of the prompt instead (merging into the
last message only when it is already a trailing user turn, i.e. plain chat).
This keeps the [system][task][tool-history] prefix stable and cache-reusable so
only the new block re-prefills, and gives the aggregator the references with
recency. Extracted as `_attach_reference_guidance` with unit tests.
Measured on a local llama.cpp aggregator over a long agentic task: KV-cache
reuse on follow-up steps went from ~0.3% to ~93-95% and per-step prefill on an
~80k-token context dropped from ~44s to <1s, with no change to output.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`_maybe_wrap_untrusted` is the architectural defense against indirect
prompt injection. It wraps attacker-controllable tool output
(web_extract, web_search, browser_*, mcp_*) in
`<untrusted_tool_result>...</untrusted_tool_result>` so the model treats
it as data. The content was interpolated verbatim, so the boundary was
forgeable.
Two holes. A poisoned page that embeds `</untrusted_tool_result>` closes
the block early — everything after it reads as trusted instructions. And
the `startswith("<untrusted_tool_result")` re-entrancy guard returned
content that merely started with the opening tag completely unwrapped, so
an attacker just prefixed the tag to drop all data framing.
Fix neutralizes any embedded delimiter token (case-insensitive) before
interpolation and drops the forgeable fast-path, so content is always
sealed in exactly one well-formed block. Re-wrapping an already-wrapped
forward is harmless — it stays framed as data.
## What does this PR do?
Closes an indirect prompt-injection bypass in the untrusted-tool-result
wrapper. Attacker content can no longer break out of, or forge, the
trust boundary.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `agent/tool_dispatch_helpers.py`: add `_neutralize_delimiters` (case-insensitive defang of the `untrusted_tool_result` token); `_maybe_wrap_untrusted` now always neutralizes then wraps, and the forgeable `startswith` re-entrancy guard is removed.
- `tests/agent/test_tool_dispatch_helpers.py`: replace the double-wrap test (it encoded the bypass) with regression tests for embedded closing tag, leading opening tag, and a cased closing tag.
## How to Test
1. `scripts/run_tests.sh tests/agent/test_tool_dispatch_helpers.py` — 29 pass.
2. Embedded `</untrusted_tool_result>` mid-content: real closing delimiter appears once, at the end; payload trapped inside.
3. Content starting with the opening tag: data framing is applied, not skipped.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run the affected tests and they pass
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15 (Darwin 25.5)
### Documentation & Housekeeping
- [x] I've updated relevant documentation (docstrings) — or N/A
- [x] cli-config.yaml.example — N/A
- [x] CONTRIBUTING.md / AGENTS.md — N/A
- [x] Cross-platform impact — N/A (pure-Python, stdlib `re`)
- [x] Tool descriptions/schemas — N/A
A literal "rm -rf /" carried as DATA inside another command's quoted
argument — a PR title, a git commit -m message, an echo/printf arg —
tripped the unconditional root-filesystem hardline and could not run at
all. `gh pr create --title "block rm -rf / spellings"` was blocked
outright, because the bare rm path branch matched the mid-string "rm"
(via \brm) with the space after "/" satisfying its (\s|$) terminator.
Anchor the shared _RM_FLAG_PREFIX to _CMDPOS so the rm hardline rules
fire only when rm is an actual command word (start of line, after a
separator ; && || |, after a subshell opener $()/backtick, or after
sudo/env/exec wrappers) — not when the string appears as an argument
value. Broaden the bare-path terminator to also accept shell
metacharacters ) ` ; | & so a real wipe inside a command substitution
is still caught.
The quoted-path branch is unchanged, so quoted root/HOME paths stay
blocked. Adds regression tests for both directions: data-arg false
positives must NOT block, real wipes at every command position must block.
Review follow-up on the batch salvage: normalize the inter-class spacing to two
blank lines (PEP8) between the three new test classes, and add an explicit
assertion in test_sanitizer_strips_orphaned_preserves_text_content that the
'(tool call removed)' placeholder does NOT overwrite existing assistant text.
No production change.
After the first compaction protect_first_n decays, so on a later compaction
the only protected head message can be the system prompt. Adapters like
Anthropic and Bedrock send the system prompt as a separate parameter, so the
summary becomes the first message in messages[] — and Anthropic rejects any
request whose first message is not role=user (HTTP 400). Pin the summary to
role=user when the head is system-only, and stop the collision-flip logic from
reverting it back to assistant.
Salvaged from #52167.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
compress() eagerly reset _last_summary_auth_failure and
_last_summary_network_failure at the top of every call. On a second
compress() during the failure cooldown, _generate_summary() returns None from
the cooldown early-return WITHOUT re-asserting those flags, so the abort guard
saw False and fell through to the destructive static-fallback that drops the
middle window — the data-loss #29559/#25585 describe. Stop resetting them
eagerly; a successful summary already clears both, so letting them persist
across calls is safe and keeps the cooldown abort protection intact.
Salvaged from #52056.
Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
_sanitize_tool_pairs inserted stub role="tool" results for orphaned
tool_calls. The pre-API repair_message_sequence() tracks known call IDs by
tc.get("id") while this sanitizer keys on call_id||id; when they disagree
(Codex Responses API: id != call_id) the stubs are silently dropped by the
repair pass, re-exposing the original orphans. Strip the orphaned tool_calls
at the source instead (preserving any text content, adding a placeholder for
an otherwise-empty assistant turn) to avoid the mismatch class entirely.
Salvaged from #51225.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
Addresses egilewski (Codex) CR on PR #52351: the run_job() credential-exfil
backstop caught every exception around _validate_cron_base_url() and set
err = None, so an unexpected validator/import error let an unvetted stored
provider/base_url pair reach resolve_runtime_provider() — the very sink this
checkpoint exists to guard. A synthetic validator-exception probe with a
legacy custom:legit + off-host base_url job slipped through (validator_exception
ALLOW).
Now fail closed: if the validator raises and the job carries a base_url
override (the exfil precondition), refuse the run. A job with no base_url
override can't exfiltrate via this path — the validator would return None — so
it still runs, keeping the common no-override jobs from wedging on an unrelated
error. Operator fallback providers come from config, not the job, so they are
unaffected.
Adds two regressions: validator-exception + base_url -> blocked;
validator-exception without base_url -> still allowed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The model-facing cronjob tool accepts free-form provider + base_url. On fire,
the scheduler pairs the named provider's stored credential with the job's
base_url, so a prompt-injected job (e.g. provider=anthropic,
base_url=https://attacker/v1) sends the real API key to an attacker endpoint. A
base_url with no provider inherits the default provider's key for the same
effect.
Add a fail-closed guard at the tool boundary: a base_url override is allowed
only for the custom/BYOK sentinel, a configured custom_providers entry, or when
the override host matches the named provider's own endpoint; an override without
an explicit provider is rejected. The trust boundary is the caller, so
operator-configured base_urls for named providers are unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `prompt.submit` handler in the TUI gateway lets a client trim the
conversation back to a chosen user turn via `truncate_before_user_ordinal`.
It validated only the upper bound (`ordinal >= len(user_indices)`) and never
the lower one. A negative ordinal therefore sailed straight past the guard and
fell into Python's negative indexing: `user_indices[-1]` resolves to the *last*
user turn, so the history was silently sliced to everything before it and that
truncated list was immediately committed to disk with `db.replace_messages`,
which deletes and reinserts the whole row in one transaction.
The impact is severe and unrecoverable: a single out-of-range value — from a
client bug, a hidden/real user-message desync, or any present or future
frontend that emits a relative ordinal — permanently destroys the user's
conversation on disk instead of returning the intended `4018` error. Because
the gateway is deliberately frontend-agnostic, it cannot assume the value is
well-formed; it must validate it.
The fix is minimal and safe: extend the existing guard to reject negatives on
the very same error path the upper bound already uses. No in-memory history is
mutated and no DB write happens for an invalid ordinal, so a bad value now
fails closed with no data loss. The valid-ordinal path is untouched.
N/A
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
- `tui_gateway/server.py`: in the `prompt.submit` handler, change the
ordinal guard from `if ordinal >= len(user_indices)` to
`if ordinal < 0 or ordinal >= len(user_indices)` so a negative ordinal is
rejected with error `4018` before any history slice or `replace_messages`
write occurs. Added a comment explaining the negative-indexing hazard.
- `tests/test_tui_gateway_server.py`: add
`test_prompt_submit_rejects_negative_truncate_ordinal`, which submits a
`truncate_before_user_ordinal` of `-1` and asserts the handler returns
`4018`, leaves the in-memory history intact, never marks the session
running, and never calls `replace_messages`. Added the `pytest` import used
by the new test's fail-fast guards.
1. Check out this branch and run
`scripts/run_tests.sh tests/test_tui_gateway_server.py -- -k negative_truncate`
— the new test passes.
2. Reproduce the bug: temporarily revert the guard to the old
`if ordinal >= len(user_indices)` and rerun — the test fails because the
handler truncates the history and starts a turn instead of returning `4018`.
3. Full file run: `scripts/run_tests.sh tests/test_tui_gateway_server.py`
(the only failure is the pre-existing, environment-dependent
`test_browser_manage_connect_default_local_reports_launch_hint`, which also
fails on clean `main` when a Chromium browser is installed locally).
- [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 — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
Upstream #52270 added `_nous_inference_env_override()` but wired it into
only `resolve_nous_runtime_credentials`. Three sibling resolution paths
still ignored the override, so a self-hosted Nous inference endpoint set
via `NOUS_INFERENCE_BASE_URL` was silently dropped whenever credentials
arrived through any of them:
- the credential-pool path (`_resolve_runtime_from_pool_entry`)
- the explicit-provider path (`_resolve_explicit_runtime`)
- the auxiliary side-LLM client (`_pool_runtime_base_url`)
Route all three through the same auth-layer reader so every
`NOUS_INFERENCE_BASE_URL` read shares one normalization path
(trailing-slash stripping, blank -> empty) and the documented
trusted-bypass intent stays in one place. The override is live-only: it
wins for the base URL returned this run but is never persisted to
auth.json or the credential pool, so an ephemeral dev/staging value
cannot poison durable auth state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What does this PR do?
A single, perfectly valid `.env` line was being silently corrupted on read
and write. When a secret's value happened to contain a known Hermes env var
name followed by `=` — for example a webhook or proxy base URL carrying a
query parameter like `OPENAI_BASE_URL=https://proxy.example.com/v1?TAVILY_API_KEY=sk-...`
— `_sanitize_env_lines()` treated the embedded `KEY=` as a second entry. It
truncated the real secret at the inner match and fabricated a bogus second
variable. A related path silently dropped any text before the first matched
key. Because this runs on every `load_env()`, `save_env_value()`,
`remove_env_value()` and `sanitize_env_file()`, the damage was written back to
`~/.hermes/.env` and re-applied on every read — persistent loss/corruption of
the canonical secrets store.
The concatenation splitter now only acts when the line actually begins with a
known `KEY=` (so leading text is never dropped) and when every value that
precedes a boundary is a plain token. If a preceding value looks structured —
a URL/query string (`://`, `?`, `&`) or contains whitespace — the embedded
`KEY=` is understood to be part of that value, and the line is kept verbatim.
Genuine concatenations of plain-token secrets still split as before.
## Related Issue
N/A
## Type of Change
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `hermes_cli/config.py`: added `_looks_like_structured_value()` helper and
reworked the split logic in `_sanitize_env_lines()` to anchor splits to the
line start and skip splitting when a preceding value looks like a URL/query
string or holds whitespace.
- `tests/hermes_cli/test_config.py`: added two regression tests — a value that
embeds a known `KEY=` is preserved verbatim, and leading text before the
first key is not dropped.
## How to Test
1. Run the sanitizer tests: `pytest tests/hermes_cli/test_config.py -k anitize -q`.
2. Confirm the new cases reproduce the bug on the old code and pass on the new:
`OPENAI_BASE_URL=https://proxy.example.com/v1?TAVILY_API_KEY=sk-embedded`
is returned unchanged instead of being split into a truncated value plus a
fabricated `TAVILY_API_KEY` entry.
3. Run the full file: `pytest tests/hermes_cli/test_config.py -q` (97 passed).
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for existing PRs 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)
### Documentation & Housekeeping
- [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 — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
The TestRunJobSessionPersistence run_job tests shared a helper that returned
a positional list of patches; callers applied a hardcoded slice
(patches[0..N]). When the BSM-seam fix split one env patch into two, the list
grew and every caller's slice silently dropped resolve_runtime_provider off
the end. The tests still passed locally — a dev machine has ambient provider
state (seeded via the cron delivery-routing path's plugin discovery) that let
the real resolver succeed — but failed on CI's clean HOME where nothing seeds
a provider, so run_job raised AuthError and AIAgent was never constructed.
Fix: _run_job_patches is now a contextmanager that enters the whole patch
bundle via ExitStack and yields (fake_db, mock_agent_cls). A caller can no
longer drop a patch by index, so a future seam change can't reintroduce the
local-green/CI-red split. Behaviour and assertions unchanged; 577 cron tests
pass.
When auxiliary.<task>.model is set to "auto" in config.yaml,
_resolve_task_provider_model() was treating it as a truthy model id
and propagating the literal string "auto" to the wire. The provider
then returned a 200 OK with an error-text body (e.g. "the model auto
does not exist, run --model to pick a different model"), which
downstream consumers such as ContextCompressor accept as the
compressed summary -- silent corruption with no exception raised.
The provider-side auto-resolution path (_resolve_auto via main_runtime
fallback) is already wired up and does the right thing when cfg_model
is None. The fix is to normalize the auto sentinel at the resolver
layer: when cfg_model.lower() == "auto", drop it to None so the
resolver can fall through to main_runtime / auto-detect.
Reproduction (pre-fix):
>>> from agent.auxiliary_client import _resolve_task_provider_model
>>> _resolve_task_provider_model("compression") # with model: auto in config
("auto", "auto", None, None, None)
Post-fix:
>>> _resolve_task_provider_model("compression")
("auto", None, None, None, None)
Verified end-to-end: ContextCompressor.compress now produces a real
summary (~4KB of compaction text) instead of swallowing the bridge
error string. Aux compression on auto/auto config no longer silently
corrupts the conversation summary.
When /compress rotates the session, the handler repointed the live
session entry onto the new (empty) continuation session_id and _save()d
that BEFORE writing the compressed transcript — and rewrite_transcript
swallowed DB write failures at DEBUG. A transient write failure (SQLite
lock under concurrent writes, ENOSPC, disk/IO error) left the session
pointing at an empty id while the handler still reported a cheerful
'Compressed: N → M' success. The active conversation vanished from view.
- gateway/session.py: rewrite_transcript now returns bool (True on write
success or no-DB, False on canonical write failure). /retry, /undo, and
yuanbao recall ignore the result, so their behavior is unchanged.
- gateway/slash_commands.py: _handle_compress_command persists the
compressed transcript FIRST and treats a write failure as fatal (raises
into the outer handler's 'compress failed' banner). Only repoints +
_save()s the session on a successful write. Widened beyond the original
rotation case to also cover in-place compaction (#38763): a failed
in-place write would otherwise leave the DB untouched while still
reporting success.
- tests: regression tests for both the rotation and in-place write-failure
paths — assert a failure banner, unchanged session_id, and no _save().
Co-authored-by: Hermes Agent <agent@nousresearch.com>
The index references the handoff_state column which is added by
_reconcile_columns() on legacy databases. Placing it in SCHEMA_SQL
causes 'no such column' errors during schema migration tests because
SCHEMA_SQL runs before reconciliation.
Move to DEFERRED_INDEX_SQL which runs after _reconcile_columns() —
matching the existing pattern used by idx_messages_session_active.
Refs: #43504, #40695
(cherry picked from commit 40ecd61d4993754e077a2bdf0c68707cd2add5f4)
The message triggers append one FTS5 segment per insert into both the
porter and trigram indexes. Nothing ever called the existing
optimize_fts() maintenance helper, so on a long-lived state.db these
segments accumulate without bound (observed: ~34k trigram segments for
~27k messages). Every MATCH then has to scan all segments, and every
insert pays a growing automerge cost that lengthens the WAL write-lock
hold time. Because the gateway and cron agents are separate processes
sharing one state.db, those longer holds exhaust the 1s-timeout x 15-retry
budget in _execute_write and surface as repeated:
Session DB creation failed (will retry next turn): database is locked
Session DB append_message failed: database is locked
Wire optimize_fts() into the write path on a coarse cadence
(_OPTIMIZE_EVERY_N_WRITES = 1000), alongside the existing every-50-writes
checkpoint. 'optimize' is effectively free once the index is already
merged, so steady-state cost is negligible; only the first merge of a
neglected index is expensive. The call is best-effort and never fails the
surrounding write.
Tests: cadence fires on the write path; a failing optimize never breaks
the write.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 583647b56e207a9b0accfd05efa2b9b251630984)
echo <base64> | base64 -d | bash (and base32/base16, xxd -r, tr
transforms, openssl base64/enc -d) decode a dangerous command at
runtime — the raw text carries no dangerous keyword, so the denylist
never fired. Adds DANGEROUS_PATTERNS entries for decode-and-execute
pipes into a shell.
eval $(curl ...), source $(wget ...), and . $(curl ...) executed
remote content but were not covered by the existing pipe-to-shell /
process-substitution patterns. Adds a DANGEROUS_PATTERNS entry so these
command-substitution forms consistently request approval.
Original authorship preserved from PR #26965 (bot-authored commit
re-attributed to the human contributor).
Command-name obfuscation bypassed the dangerous-command denylist: the
executable name could be spelled with shell tricks that survive regex
matching but still resolve to a blocked command at runtime —
$(echo rm), ${0/x/r}m, backticks, and printf substitutions.
Adds a non-executing shell-word scanner that deobfuscates only at
command positions (start, after ;|&&||, inside $(...), after
sudo/env/exec/... wrappers) and feeds the resulting variants through
the existing HARDLINE_PATTERNS / DANGEROUS_PATTERNS — no second
blocklist. Scoping to command words keeps ordinary arguments
(echo $(echo rm) -rf /) from being promoted into command names.
Co-authored-by: egilewski <1078345+egilewski@users.noreply.github.com>
`_normalize_command_for_detection` strips backslash-escapes before matching
DANGEROUS_PATTERNS and HARDLINE_PATTERNS, but the strip rule was
`re.sub(r'\\([^\n])', r'\1', ...)` — its `[^\n]` class deliberately skips
newlines. A backslash immediately followed by a newline is a POSIX line
continuation: the shell removes BOTH characters and joins the tokens, so
`rm -rf \<newline>/` executes as `rm -rf /`. With the dangling backslash left
in place, the structured rm/dd/mkfs patterns no longer match because a literal
`\` sits wedged between the tokens they expect to be adjacent.
The worst consequence is on the HARDLINE floor. The dangerous-command layer
still fired here only by accident (the generic `\brm\s+-[^\s]*r` "recursive
delete" rule needs no path), and that layer is bypassed by `--yolo` /
`approvals.mode=off`. The hardline blocklist — the unconditional floor reserved
for catastrophic, unrecoverable commands and meant to hold even under yolo —
anchors the root path directly after the flags, so `rm -rf \<newline>/`,
`rm -r\<newline>f /`, and `rm -rf \<newline>~` all slipped past it entirely.
A yolo session could therefore wipe the root filesystem.
The fix collapses line continuations (`\` + `\n` or `\r\n`) to nothing,
mirroring the shell, before the existing escape strip runs. This was the gap
left by 621bf3a87, which added the escape strip but only for non-newline chars.
## What does this PR do?
Closes a shell line-continuation bypass in the dangerous-command detector.
Before: `rm -rf \<newline>/` normalized to `rm -rf \<newline>/`, so the
hardline root-delete patterns did not match and the command could run under
`--yolo`. After: line continuations are collapsed first, the command
normalizes to `rm -rf /`, and the hardline floor blocks it unconditionally.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: in `_normalize_command_for_detection`, add
`command = re.sub(r'\\\r?\n', '', command)` ahead of the existing
backslash-escape strip so shell line continuations (`\`+newline, LF or CRLF)
are removed exactly as the shell would, instead of leaving a stray backslash
that breaks the structured patterns.
- `tests/tools/test_hardline_blocklist.py`: add a parametrized
`test_hardline_blocks_line_continuation` covering the root, in-flag, home,
CRLF, and mkfs continuation forms, plus
`test_line_continuation_root_wipe_cannot_bypass_hardline` asserting the
continuation root wipe stays blocked even with `HERMES_YOLO_MODE=1`.
## How to Test
1. Reproduce: stash the `tools/approval.py` change and run
`scripts/run_tests.sh tests/tools/test_hardline_blocklist.py` — the new
line-continuation cases fail (`rm -rf \<newline>/` is not flagged hardline,
and leaks past the floor under yolo).
2. Restore the change and rerun the file — all 106 tests pass.
3. Regression: `scripts/run_tests.sh tests/tools/test_approval.py` (the
existing fullwidth/ANSI/null-byte normalization and multiline cases still
pass).
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for existing PRs 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)
### Documentation & Housekeeping
- [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) — handles both LF and CRLF line endings
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
# Conflicts:
# tools/approval.py
When an Anthropic Claude Pro/Max OAuth subscription hits the "out of extra
usage" 400 (now classified as billing), surface actionable guidance pointing
at claude.ai/settings/usage and the cycle-reset option instead of the generic
"add credits with that provider" line — which does not apply to a
subscription. Folds in the UX from #40073 (@harsh-matchmyflight) without the
extra FailoverReason enum; the billing reclass already provides the recovery
behavior.
Anthropic returns HTTP 400 with "You're out of extra usage. Add more at
claude.ai/settings/usage and keep going." when the account's extra-usage
allowance is depleted. The existing _BILLING_PATTERNS list did not
include this wording, so classify_api_error fell through to generic
format_error — non-retryable and should_fallback=False — causing the
agent to abort instead of engaging the configured fallback chain.
Add the pattern and a regression test covering the exact Anthropic body.
Remove scripts/setup_open_webui.sh and its 'one-command local bootstrap'
doc sections (EN + zh-Hans). The script pip-installed the third-party Open
WebUI frontend into ~/.local and managed a launchd/systemd user service —
a maintenance liability for downstream software we don't own, and the source
of the LAN first-admin signup footgun in #36121.
The Open WebUI *integration* via the OpenAI-compatible API server is
unaffected: the Docker/Docker-Compose setup, multi-user profile guide, and
troubleshooting in open-webui.md stay, and Open WebUI remains a listed
supported frontend. Only the install-and-service bootstrapper is gone.
A finite one-shot cron job whose side effect kills the tick (gateway
suicide, OOM, segfault, hard-timeout) re-fired forever: mark_job_run —
which increments repeat.completed and removes the job — runs AFTER the
job, so an abrupt tick death never records completion and every
supervisor relaunch re-dispatches the job (#38758).
Commit the dispatch BEFORE the side effect:
- claim_dispatch() increments repeat.completed under the cross-process
jobs lock and persists it before run_job(), converting finite
one-shots from at-least-once to at-most-times.
- Called from run_one_job (the shared body used by BOTH the built-in
ticker and the external Chronos fire_due path) before run_job.
- mark_job_run skips the increment for pre-claimed one-shots (no
double-count) and still removes at the limit.
- get_due_jobs drops a stale one-shot already at its dispatch limit so
a job claimed-but-not-cleaned-up after a crash stops appearing as due.
- No-op for recurring jobs (advance_next_run) and infinite/no-repeat
one-shots; a handed-in job dict absent from the store proceeds.
Closes#38758
The salvaged write-target boundary included `#` in its char class, so a
`#` glued to the redirect/tee path (`echo x > .env#backup`) matched as a
comment boundary and flagged the write as dangerous. But the shell writes
to the distinct file `.env#backup`, not `.env` — a false positive, same
class as the config.yaml.bak case the PR already excluded. Drop `#` from
the boundary; a real trailing comment is always whitespace-preceded (\\s).
Adds regression tests for .env#backup, config.yaml#backup, and
tee .env#backup staying out of the deny.
The dangerous-command approval gate has rules that flag a shell command
when it overwrites a project `.env` or `config.yaml` — these files hold
API keys, DB passwords, and (for `config.yaml`) the approval policy
itself, so a write to them should require user approval. The matching
`write_file`/`patch` deny on the file-tools side was paired with these
terminal-side rules so neither path is an open door.
The redirection and `tee` rules anchored the sensitive path with
`_COMMAND_TAIL` (`(?:\s*(?:&&|\|\||;).*)?$`), which only tolerates the
rest of the line being empty or a command separator. The problem: in
POSIX shell the redirection target is fixed regardless of what trails it.
`echo secret > .env extra` still truncates `.env` (the `extra` is just
another argument to `echo`), and `echo secret > .env # note` does too
(the `#` starts a comment). Because neither tail is a separator, the old
anchor failed to match and the command sailed through approval — a
prompt-injected step could overwrite a project `.env`/`config.yaml`
unprompted. The system-path redirection rule one line above never had
this restriction and already caught these forms.
The fix introduces `_WRITE_TARGET_BOUNDARY`, a lookahead that only
requires the path token to END at a shell word boundary (whitespace,
quote, separator, redirection operator, `#`, or EOL) rather than
demanding the rest of the line be empty. It is applied to the two
stream-write rules (redirection and `tee`) where the sensitive path is
always a write target. The `cp`/`mv`/`install` rule deliberately keeps
`_COMMAND_TAIL`: there the sensitive file is only a target when it is the
LAST argument (the destination), so requiring end-of-line is correct and
keeps `cp config.yaml backup.yaml` (config.yaml as the source) out of the
deny.
## What does this PR do?
Closes a bypass in the dangerous-command approval gate where a trailing
argument or `#` comment after a `>`/`>>`/`tee` write target let a command
overwrite a project `.env` or `config.yaml` without triggering approval,
even though the shell still overwrites the file.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: add `_WRITE_TARGET_BOUNDARY` (a word-boundary
lookahead) and use it instead of `_COMMAND_TAIL` in the two
project-env/config stream-write patterns ("overwrite project env/config
via tee" and "via redirection"). `_COMMAND_TAIL` is kept and still used
by the `cp`/`mv`/`install` rule, where end-of-line anchoring is the
correct semantics.
- `tests/tools/test_approval.py`: add regression tests for
`> .env extra`, `> .env # note`, `>> config.yaml foo`, and
`tee .env backup` (now flagged), plus `> config.yaml.bak` (must stay
safe — different file).
## How to Test
1. Reproduce: before the fix,
`detect_dangerous_command("echo secret > .env extra")` returns
`(False, None, None)` — the overwrite is not flagged.
2. Apply the fix; the same call now returns the "overwrite project
env/config via redirection" detection.
3. Run `pytest tests/tools/test_approval.py -q` — the new cases pass and
the existing `cp config.yaml backup.yaml` / `config.yaml.bak`
false-positive guards still hold.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run the relevant tests and they pass
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15 (Darwin 25.5)
### Documentation & Housekeeping
- [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) — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
