A plugin-registered WebSearchProvider with no built-in provider credentials
must light up web_search / web_extract and be discoverable by the backend
selectors. Covers check_web_api_key(), _get_backend(), _is_backend_available()
registry delegation, per-capability extract selection (#32698), and that the
web_search / web_extract tool registry entries are not filtered out.
Tests contributed by @m0n5t3r (PR #28652, issue #28651).
The salvaged tests from #53754 predate _handle_vision_analyze becoming
async and the native fast path; await the handler and force the legacy
aux path so the model-resolution assertion is actually exercised.
_handlers for vision_analyze and video_analyze read model name from
config.yaml (auxiliary.vision.model / auxiliary.video.model) before
falling back to AUXILIARY_VISION_MODEL / AUXILIARY_VIDEO_MODEL env
vars. Matches the existing config-first pattern for timeout and
temperature in the same file.
Fixes#53749
A user with tts.openai.model set to a direct-OpenAI model (e.g. tts-1-hd)
but no VOICE_TOOLS_OPENAI_KEY/OPENAI_API_KEY (or with tts.use_gateway)
routes TTS through the managed Nous audio gateway, which only proxies
gpt-4o-mini-tts. The request 400s with:
VALIDATION_ERROR: Unsupported managed OpenAI speech model
{'model': 'tts-1-hd', 'supportedModels': ['gpt-4o-mini-tts']}
_resolve_openai_audio_client_config now reports whether it resolved the
managed gateway; _generate_openai_tts coerces the model to a
managed-supported one (logging a warning that points at the direct-key
escape hatch) unless the user redirected base_url to their own endpoint.
Direct-key users keep their tts-1/tts-1-hd preference unchanged.
Source: https://github.com/NousResearch/hermes-agent/pull/52346
Related prior work: https://github.com/NousResearch/hermes-agent/pull/39462
Related prior work: https://github.com/NousResearch/hermes-agent/pull/27426
Maintainer direction: https://github.com/NousResearch/hermes-agent/pull/52346#issuecomment-4854881612
Remove acp_command and acp_args from the model-facing delegate_task schema and
dispatch paths. Child agents can still use ACP subprocess transport when it
comes from trusted delegation config or parent inheritance, but a model tool
call can no longer choose the command or arguments that reach child
construction.
This is salvageable because the risky boundary is model control over child ACP
transport, not ACP itself. The patch follows the maintainer direction from the
source discussion by preserving trusted ACP configuration and prior integration
work while removing the untrusted tool-call fields from both top-level and
per-task delegate inputs.
Reproduced on main by passing acp_command through delegate_task and observing it
reach _build_child_agent. Verified after the fix that model dispatch strips the
hidden top-level fields and per-task hidden fields are ignored before child
construction.
Co-authored-by: Carlosian <claudlos@agentmail.to>
Co-authored-by: ssiweifnag <120658181+ssiweifnag@users.noreply.github.com>
Co-authored-by: nikshepsvn <23241247+nikshepsvn@users.noreply.github.com>
browser_cdp's frame_id (OOPIF) path returned early via
_browser_cdp_via_supervisor before _browser_cdp_private_guard ever ran,
unlike the stateless path a few lines below. A model that navigated a
cloud browser to a private/internal URL could still read page content
by passing frame_id, bypassing the same SSRF/private-page boundary
already enforced on Runtime.evaluate, Page.navigate, and other raw CDP
calls.
Apply the same guard call used by the stateless path before dispatching
to the supervisor, so both routing modes share one boundary.
MSYS_NO_PATHCONV is honored by Git for Windows bash only. _find_bash's
final shutil.which fallback can return MSYS2-proper or Cygwin bash,
which ignore it and honor MSYS2_ARG_CONV_EXCL instead. Set both so argv
path conversion stays disabled regardless of which bash flavor spawns.
Also subsumes the cmd /c mangling in #56147.
Follow-up to #56874, which added the Camofox private-page SSRF guard
(_camofox_current_page_private_url) but wired it only into the Camofox
eval path (_camofox_eval). The other Camofox content-read tools —
camofox_snapshot, camofox_get_images, and camofox_vision — still read the
current page's accessibility tree / images / screenshot without the
guard, so on a non-local Camofox backend they can return the content of
an intranet or cloud-metadata page (e.g. 169.254.169.254) that the
terminal itself can't reach.
Apply the same guard, gated on _eval_ssrf_guard_active (non-local
backend, not a local sidecar, allow_private_urls unset) and fail-open on
probe failure, matching the eval-path guard and the main-browser
snapshot/vision guards. camofox_back is intentionally not changed: its
target is unknown until navigation completes, and the subsequent content
read is already guarded.
Adds regression tests covering the three read tools blocking on a private
page, the public-page pass-through, and the guard-inactive no-probe path.
Three CLI reliability fixes:
1. Interrupt reliability: chat() only re-queued the user's interrupt
message when the turn result carried interrupted=True. When the agent
thread raced past its last interrupt check (or finished) before the
interrupt landed, the message was silently dropped — and the stale
_interrupt_requested flag left on the agent instantly aborted the
NEXT turn. Un-acknowledged interrupt messages are now re-queued as
the next turn and the stale flag is cleared (only when the agent
thread actually exited). The clarify-race path also parks the message
in _pending_input instead of dropping it.
2. Slow exit (5+ min): stdlib ThreadPoolExecutor workers are non-daemon
and joined unconditionally by concurrent.futures' atexit hook — even
after shutdown(wait=False). One wedged tool worker (abandoned after
interrupt/timeout) held the process open forever. Promoted
async_delegation's daemon executor to a shared tools/daemon_pool
module and adopted it in tool_executor (concurrent tool batches),
memory_manager (background sync), delegate_tool (child timeout wrapper
+ batch fan-out), and skills_hub (source fan-out). Added a 30s exit
watchdog (HERMES_EXIT_WATCHDOG_S) armed at _run_cleanup start as a
backstop for wedged cleanup steps.
3. Exit jank: after prompt_toolkit tears down the input/status bars the
terminal sat silent for the whole cleanup window, looking hung. Print
'Shutting down… (finalizing session)' immediately at exit start.
E2E: live PTY interrupt of a foreground 'sleep 120' terminal tool now
aborts in ~1s and the typed message runs as the next turn; wedged-worker
+ wedged-cleanup subprocess exits in 5.8s (watchdog) instead of hanging.
delegation.max_concurrent_children is now the single cap for both a
batch's parallelism and concurrent background delegation units.
- _get_max_async_children() delegates to _get_max_concurrent_children();
a leftover max_async_children key logs a one-time deprecation warning
- config v32→33 migration removes the stale key, folding a raised
max_async_children into max_concurrent_children (max wins, no lost
headroom)
- capacity error messages now point at max_concurrent_children
- pool-at-capacity sync fallback now attaches an explanatory note so
the model/user know why the call blocked instead of dispatching async
Previously users who raised max_concurrent_children (e.g. to 15) still
hit the invisible default-3 async cap: the 4th background delegate_task
silently ran inline, blocking the turn with no signal.
CLAUDE_CODE_OAUTH_TOKEN is set and owned by the user's Claude Code
install (subscription OAuth), not a Hermes-managed inference
credential — Claude subscription auth is not a working Hermes provider
path. Blocklisting it broke agent-spawned claude CLIs: with no token in
the child env, claude fell through to the shared macOS Keychain /
~/.claude/.credentials.json store and, on auth failure, cleared it —
logging the user out of their interactive Claude sessions and the
desktop app.
Exempt it from _HERMES_PROVIDER_ENV_BLOCKLIST (it arrives via the
anthropic registry entry, so discard explicitly with rationale).
ANTHROPIC_API_KEY / ANTHROPIC_TOKEN and every other provider credential
remain stripped, and the GHSA-rhgp-j443-p4rf fail-closed passthrough
guard is unchanged for everything still on the blocklist.
Fixes#55878
Extends the browser private-network eval guard to the Camofox backend.
On main, _browser_eval() returned early in Camofox mode before running the
shared private-URL literal pre-scan and before re-checking the page URL
after eval, leaving Camofox as a sibling backend that could execute
browser_console(expression=...) against private/internal targets.
- move the eval private-URL literal pre-scan before the Camofox early return
- add a Camofox current-page private-URL probe via the evaluate endpoint
- withhold Camofox eval results when the page is now private/internal
Follow-up to browser private-network hardening in #56173, #56526, #56664.
Salvage of #56764 by @rayjun (rayoo), cherry-picked to preserve authorship.
Vertex AI authenticates via OAuth2 (service-account JSON path / ADC), not
PROVIDER_REGISTRY, and VERTEX_CREDENTIALS_PATH is declared with
password=False (it's a path, not a bare key) under category="provider" —
a category the registry-derived blocklist loop never checks. Both it and
GOOGLE_APPLICATION_CREDENTIALS (the ADC fallback the adapter also reads)
fell through every existing blocklist source and leaked the on-disk
location of a GCP service-account key into every spawned subprocess
(terminal, codex/copilot app-server, browser workers) — the same leak
class already closed for every other provider's credentials in #53503.
Every other content-returning browser tool entry point
(browser_snapshot/vision/console/eval, and click/type/press via
_blocked_private_page_action) re-checks window.location.href against the
private/internal/cloud-metadata floor after the page could have changed --
because a redirect chain or client-side navigation can land on an address
the initial browser_navigate preflight never saw. browser_back was the one
navigation-triggering entry point missing this: it called
_run_browser_command(..., "back", []) and returned the resulting URL
straight to the model with no re-check.
On a cloud/CDP (non-local) backend, if browser history contains a
private/internal address (e.g. a prior redirect touched an internal host),
browser_back would navigate the live browser there and hand the URL back
to the model with no guard -- the exact class of gap the private-page
guard exists to close, just on the one entry point it hadn't reached yet.
Re-check happens after the navigation succeeds (not before, unlike
click/type/press) since it's the resulting page -- not the one being left
-- whose safety matters. A failed back navigation (no history) skips the
check entirely since nothing changed. Verified live: the new regression
test fails (returns the private URL instead of a blocked payload) on the
pre-fix code and passes after.
The Windows _quote_cwd_for_cd override only reached _wrap_command; the
snapshot bootstrap cd in init_session still used a bare shlex.quote(),
so on Windows the bootstrap cd failed and pwd -P captured the login
shell's dir instead of terminal.cwd. Route it through _quote_cwd_for_cd
too, and add -- for hyphen-safety to match _wrap_command.
The model could pass `toolsets` (top-level and per-task) to delegate_task,
letting it choose which toolsets a subagent got. Toolset selection is a
capability-scoping decision the model should not control; subagents inherit
the parent's enabled toolsets, period.
- Remove `toolsets` from the delegate_task() signature, the registry handler,
the top-level + per-task JSON schema, and the live dispatch path
(run_agent._dispatch_delegate_task — this forwarded it on every model call).
- Single-task and per-task child builds now pass toolsets=None so
_build_child_agent resolves to pure parent inheritance.
- Drop the now-dead _SUBAGENT_TOOLSETS / _TOOLSET_LIST_STR schema-hint block.
- _build_child_agent keeps its internal toolsets param + intersection helpers
(internal API; fed the inherited value only).
- Tests: schema assertions flipped to assertNotIn; added a regression test
proving the dispatch path never forwards a smuggled model `toolsets`.
- Docs: update delegate_task signature refs in the autonomous-ai-agents skill.
The patch tool's strategy 7 (unicode_normalized) matches ASCII old_string
against a file containing real Unicode (em-dashes, smart quotes, ellipsis,
non-breaking spaces). Writing new_string verbatim silently replaced the
file's Unicode with the LLM's ASCII equivalents.
_preserve_unicode_in_replacement() diffs old_string->new_string and applies
only the actual edits to the file's original Unicode text, preserving
unchanged characters.
Salvaged from #50540 by @aj-nt. Only the Unicode-preservation half is
carried over; the write_file line-number-strip half was dropped (the
existing _looks_like_read_file_line_numbered_content reject guard already
covers its target case, and the strip's looser threshold risks silently
mutating legitimate pipe-delimited content).
browser_navigate's always-blocked cloud-metadata floor (169.254.169.254,
metadata.google.internal, ECS/Azure/GCP IMDS) was gated on
`not _is_local_backend()`, contradicting both the adjacent comment and the
is_always_blocked_url docstring ("denied regardless of backend"). A default
local headless Chromium on a cloud VM — or an off-host CDP browser — could
navigate to IMDS and read instance credentials into the model context. Make the
floor unconditional on the initial-nav and post-redirect paths.
Also: _is_local_backend() ignored a CDP override while _is_local_mode() honors
it, so an off-host CDP browser was treated as "local" and skipped the broader
private/internal SSRF check too. Treat a CDP override as non-local.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up on the salvaged #49830 hardening. The contributor's sensitive
query-param set included bare English words (code, key, auth, session,
sig) that double as ordinary page facets — ?code= on promo/challenge
pages, ?key= as a search facet, ?session= on blogs — so web_extract and
cloud browser_navigate would refuse a large slice of normal browsing.
Narrow the set to unambiguously credential-named params (access_token,
authorization, client_secret, password, token, x-amz-signature, ...).
Prefix-based vendor-key redaction (is_safe_url) still catches recognizable
key shapes; this set is the belt-and-suspenders for opaque secrets carried
under an explicit credential-named parameter.
Also fixes two intra-PR-staleness test breakages surfaced by salvaging onto
current main:
- web_extract_tool() no longer accepts use_llm_processing= (signature
changed since the PR was authored) — dropped the invalid kwarg.
- agent.redact now fully masks keyed 'token=<secret>' to 'token=***'
instead of partial 'sk-...'; the console-redaction test now asserts the
real invariant (secret body gone) rather than the exact mask format.
Added a regression test that generic English-word query params are NOT
blocked by the credential guard.
Add policy gates and output redaction for browser/CDP surfaces, strengthen session ownership tracking, and block credential-like query parameters before third-party browser/web backends receive URLs.
Inspired by the agbrowse review: keep local browser magic-link flows possible while preventing cloud reader/browser escalation from receiving opaque token, code, signature, or key query parameters.
Root cause: gateway spawns LSP servers (jdtls/pyright/yaml-ls) and
slash_worker without start_new_session=True, so they inherit the
gateway process group (= TUI parent PID). When mcp_tool
_snapshot_child_pids() races with these spawns during stdio MCP
server startup, non-MCP children leak into _stdio_pgids with the
TUI parent PGID. shutdown_mcp_servers() then killpg(tui_parent_pid,
SIGTERM), killing the TUI itself.
Evidence: tui_gateway_crash.log shows recurring SIGTERM stacks:
shutdown_mcp_servers -> _kill_orphaned_mcp_children ->
_send_signal -> killpg(pgid, sig) -> SIGTERM received
Fix (3 layers):
1. agent/lsp/client.py: add start_new_session=True to LSP server
spawn so each LSP server gets its own process group/session.
2. tui_gateway/server.py: same fix for slash_worker spawn, the
symmetric root-cause patch so no gateway direct child shares
the TUI parent pgid.
3. tools/mcp_tool.py: add _filter_mcp_children() defense-in-depth
that drops non-MCP children (slash_worker, jdtls/eclipse LSP)
from the PID delta before they can poison _stdio_pgids.
git's and sudo's option parsers resolve unambiguous long-flag prefixes, so
`git reset --har`, `git branch --delete --force`, and `sudo --stdi`/`--ask`
execute identically to their full-flag forms while evading the exact-string
DANGEROUS_PATTERNS regexes that gate them. Verified live against real git
and sudo binaries. Widen the patterns to accept unambiguous abbreviations,
scoped narrowly enough to avoid colliding with sibling flags (--help,
--soft/--mixed/--merge/--keep, --shell/--set-home).
Rework follow-up on the Windows destructive-shell detection. The PowerShell
pattern required an explicit -Command/-c before the verb, but PowerShell runs
the verb as the DEFAULT POSITIONAL arg — so `powershell Remove-Item -Recurse
-Force C:\x` (no -Command) slipped through, the exact case the PR body claims
to close. Also missing the canonical `ri` alias.
Anchor the verb to the command position (after the shell name + any leading
-Flag switches + optional -Command/-c) so bare invocations are caught while a
benign path arg containing 'del'/'rm' (e.g. -File c:\del-logs\run.ps1) is not.
Add ri to the verb list. Mutation-verified regression tests for the bare
invocation, ri alias, and the benign-path negative.
The execute_code sandbox exposed its tool-call RPC (AF_UNIX socket and
remote file-poll transports) without any caller check, so any local
process that could reach the socket / rpc dir could dispatch
terminal-capable tool calls through the parent. Mint a per-session
HERMES_RPC_TOKEN, pass it to the sandboxed child, and require a
timing-safe match on every request in both _rpc_server_loop and
_rpc_poll_loop. Empty/missing/wrong token fails closed.
Salvaged from #44073 (per-session RPC token). Added timing-safe
secrets.compare_digest comparison and fail-closed regression tests.
Co-authored-by: Hermes Agent <agent@nousresearch.com>
Review of the #50531 salvage found the cross-session HERMES_SESSION_* leak also
survives on the non-terminal spawn helper hermes_subprocess_env (added by #56202
after #50531 was written), which does os.environ.copy() without the guard. Of
its six callers, five re-bind the session identity explicitly (slash_worker/ACP
via --session-key argv) and are safe by accident; but tui_gateway cli.exec
(server.py) spawns a fresh CLI with NO --session-key under the engaged TUI host,
so it inherits a possibly-foreign HERMES_SESSION_* from the last-writer-wins
global and would stamp Kanban rows / telemetry with another session's id.
Route hermes_subprocess_env through the same _inject_session_context_env
chokepoint, restoring the single-uniform-policy-across-every-spawn-surface
invariant the codebase already claims for the internal-secret filter. Safe for
all six callers: bound ContextVars win (re-binders unaffected), _UNSET strips
(closes cli.exec). Adds 3 guard tests; mutation-checked.
Session vars (HERMES_SESSION_*) have a process-global os.environ mirror written
last-writer-wins as a CLI/cron fallback and never cleared. Under a concurrent
multi-session host (messaging gateway, ACP adapter, API server, TUI) that global
belongs to whichever turn wrote it last. A subprocess spawned from a task whose
session ContextVar is _UNSET (a sibling task that never bound, or one that
inherited another session's context) inherited the FOREIGN global and acted on
another session's identity.
Add a session_context_engaged() latch (set once any host calls set_session_vars)
and route both terminal spawn paths through a single _inject_session_context_env
chokepoint: once engaged, a bound ContextVar (incl. "") is authoritative and an
_UNSET var is STRIPPED rather than inheriting the possibly-foreign global. Pure
single-process CLI/one-shot (never engaged) keeps the inherited fallback.
Salvaged from #50531 (supersedes #49922). local.py hunk re-applied by intent
onto the current hermes_subprocess_env refactor.
Co-authored-by: PolyphonyRequiem <3107779+PolyphonyRequiem@users.noreply.github.com>
_plugin_override_policy is keyed by the plugin package root
(e.g. hermes_plugins.allowed), but the lookup used caller_mod
(the exact leaf module string). A call from hermes_plugins.allowed.cleanup
would evaluate _plugin_override_policy.get("hermes_plugins.allowed.cleanup")
→ False and raise PermissionError even when the plugin registered opt-in
under its package root.
Switch the policy lookup to caller_root (.join of the first two segments)
so submodule callers inherit the package-level allow_tool_override grant.
Adds a focused regression test for the opted-in submodule case.
Wrapping a catastrophic command in a bare subshell or brace group walked
straight past the unconditional hardline floor -- even under --yolo,
/yolo, approvals.mode=off, and cron approve mode. The command-substitution
forms were already caught; the bare paren / brace-group forms were the gap.
Rather than add the paren and brace openers to the flat _CMDPOS pattern
class (which cannot tell a real subshell opener from one sitting inside a
quoted argument, and would false-positive on ordinary prose such as a PR
title that merely mentions the trigger word), teach the existing
QUOTE-AWARE command-start tokenizer (_iter_shell_command_starts) to treat
the paren and brace openers as command starts, then emit a detection
variant that marks each real command start with a newline (already a
_CMDPOS separator). Openers inside quotes never register as starts, so
quoted arguments are left untouched while real subshell/brace bypasses now
anchor. One place covers every _CMDPOS rule (shutdown/reboot/init/
systemctl/telinit and the rm root/home/system floor).
Tests: subshell/brace bypasses added to the hardline-block, root-wipe, and
yolo-bypass sets; a regression set asserts quoted paren/brace prose is NOT
blocked (guards our own gh-pr-create workflow).
rm -rf //, /., /./, /.. and //* all resolve to / in the shell but slipped
past the root-filesystem hardline pattern, whose target group only matched
the literal / and /* tokens. They fell to the softer DANGEROUS_PATTERNS
'delete in root path' rule, which --yolo / approvals.mode=off / cron
approve-mode are designed to bypass — leaving the one unconditional floor
open to a full root wipe under yolo.
Broaden the root token from '/|/\\*|/ \\*' to '/[/.]*\\**' inside
_hardline_rm_path so any root-anchored path whose components collapse back
to / (repeated slashes plus ./.. segments) with an optional trailing glob
is caught. A trailing real segment (/tmp, /home, /.ssh) still fails to
match and stays with the softer rules.
Co-authored-by: kernel-t1 <214165399+kernel-t1@users.noreply.github.com>
auxv leaks AT_RANDOM (stack canary seed) + AT_BASE/AT_PHDR load
addresses — an ASLR oracle on par with maps. pagemap exposes
virtual->physical translation. Both slipped through the endswith
tuple alongside the maps family covered by the salvaged commit.
Adds regression coverage for auxv/pagemap and for the per-thread
/proc/<pid>/task/<tid>/<file> alias form (endswith catches both).
Follow-up on #32238, closes#34430.
PR #4609 blocked /proc/*/maps to prevent ASLR layout leakage, but the
endswith("/maps") check does not match /proc/*/smaps or
/proc/*/smaps_rollup — both expose the same virtual-address layout and
bypass the guard. /proc/*/numa_maps carries the same data with NUMA
annotations and is equally bypassed. /proc/*/mem (raw process memory)
is added as defence-in-depth; it requires address knowledge to exploit
but is blocked for consistency.
Extends the endswith tuple in _is_blocked_device_path() to cover all
four variants and adds regression assertions for all new paths to
test_proc_sensitive_pseudo_files_blocked.
Partially addresses #4427.
## What does this PR do?
Closes a critical bypass of the dangerous-command approval system. The
normalizer that every command passes through before pattern matching
(`_normalize_command_for_detection`) already strips ANSI, null bytes,
fullwidth Unicode, backslash escapes and empty-quote token splits — but
it did nothing about the shell `IFS` variable. In any POSIX shell `$IFS`
and `${IFS}` expand to whitespace, so a command written as
`rm${IFS}-rf${IFS}/` is executed by the live shell as `rm -rf /` while
the detection regexes — which anchor on literal `\s` between a command and
its arguments — never fire.
The impact is severe: this evades BOTH layers at once. It slips past every
entry in `DANGEROUS_PATTERNS` (so `curl${IFS}...|sh`, `sed${IFS}-i`
against `~/.hermes/config.yaml`, sudo privilege flags, etc. auto-run with
no approval prompt) AND the unconditional hardline floor that is
documented as un-bypassable "not even with --yolo" (`rm -rf /`, `mkfs`,
`dd` to a raw block device, `shutdown`/`reboot`, fork bomb). A
prompt-injected or malicious instruction could wipe the host filesystem or
power the box off while the approval system reports nothing. Confirmed at
runtime before the fix: `detect_hardline_command('rm${IFS}-rf /')` returned
`(False, None)`.
The fix mirrors the shell's own expansion: it collapses `$IFS` / `${IFS}`
(including the bash substring form `${IFS:0:1}`) to a single space inside
the existing de-obfuscation block, so the whitespace-anchored patterns
match exactly as they do for the un-obfuscated command. It is deliberately
narrow and safe — a `\b` word boundary keeps it from touching unrelated
variables like `$IFSACONFIG`, so it cannot introduce false positives on
legitimate commands.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: in `_normalize_command_for_detection`, substitute
`$IFS` / `${IFS}` (and `${IFS:...}`) expansions with a literal space
before dangerous/hardline pattern matching, alongside the existing
backslash and empty-quote de-obfuscation.
- `tests/tools/test_approval.py`: add `TestIFSWhitespaceBypass` covering
the brace, bare and substring IFS forms against both
`detect_hardline_command` and `detect_dangerous_command`, plus
regression guards that a look-alike variable (`$IFSACONFIG`) and plain
safe commands are not flagged. Import `detect_hardline_command`.
## How to Test
1. Reproduce the hole (pre-fix): `detect_hardline_command('rm${IFS}-rf /')`
returns `(False, None)` and `detect_dangerous_command(...)` returns
`(False, ...)`, i.e. a host-destroying command is auto-approved.
2. With the fix applied, both now flag the command: hardline match
"recursive delete of root filesystem" and dangerous match "delete in
root path".
3. Run the suite: `pytest tests/tools/test_approval.py
tests/tools/test_hardline_blocklist.py -q` — the new
`TestIFSWhitespaceBypass` cases pass and nothing else regresses.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(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 (no unrelated commits)
- [x] I've run the relevant tests and they pass (two pre-existing failures
are environmental: missing optional deps in the minimal venv, not
caused by this change)
- [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) — the change is a
pure string transform with no platform-specific behavior; footgun gate passes
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
_strategy_exact advanced its scan cursor by pos+1 instead of
pos+len(pattern), so self-overlapping patterns (e.g. "aa" in "aaaa")
matched at overlapping offsets. _apply_replacements works in reverse
order, so the second replacement operated on already-modified content
using stale offsets — corrupting the file and reporting the wrong count
under replace_all=True. Advancing by len(pattern) matches str.replace()
semantics.
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>
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.
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>
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
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
Review follow-up on the private-page action guard:
- Add test_guard_inactive_does_not_block_or_probe: when the SSRF guard is
inactive (local backend / allow_private_urls), click/type/press must proceed
WITHOUT probing the page URL. This is the branch most likely to silently
regress if the guard condition is inverted; a mutation check (flipping the
condition) confirms the test fails as designed.
- Add test_camofox_short_circuits_before_guard: camofox mode returns from the
dedicated camofox_* path before the guard runs; guards never consulted.
- Fix PEP8: 3 -> 2 blank lines before _blocked_private_page_action.
