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.
## What does this PR do?
Closes a critical hole in the hardline command floor. HARDLINE_PATTERNS is
the unconditional last line of defense: detect_hardline_command runs BEFORE
every yolo / approvals.mode=off / cron approve-mode bypass, so it is the only
gate standing between the agent (or a prompt-injected instruction) and an
irrecoverable disk wipe. The three rm rules anchored on a bare path token,
and _normalize_command_for_detection never strips shell quotes — so the
ordinary, recommended shell idioms slipped straight through:
rm -rf "/" rm -rf '/' rm -rf "/etc"
rm -rf "$HOME" rm -rf ${HOME} rm -rf "${HOME}"
All of these returned NO hardline match. A leading quote pushes the path out
of reach of the flag group, a trailing quote breaks the `(\s|$)` terminator,
and the `${HOME}` brace form was never listed at all. Under --yolo,
approvals.mode=off, or cron approve-mode the dangerous-command layer is also
skipped, so these commands reached execution with zero gate — exactly the
unrecoverable data loss the floor is documented to make impossible. Because
quoting paths and `${HOME}` are normal shell usage, not exotic obfuscation,
this is a high-severity, easily-triggered bypass.
The fix makes the rm path matcher quote- and brace-tolerant while staying
conservative: a path is matched when it is either fully wrapped in its own
matching quote pair (`"/"`) or bare with a whitespace/end terminator. The
matching-quote requirement is deliberate so the change adds no new false
positives — a dangerous-looking string that is merely an argument to another
command (e.g. `git commit -m "rm -rf /"`) has a closing quote but no opening
quote of its own around the path, so neither branch fires.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: added `_hardline_rm_path()` (matches a destructive
path either fully quoted or bare-with-terminator), factored the protected
system-dir list into `_HARDLINE_SYSTEM_DIRS` and the rm flag prefix into
`_RM_FLAG_PREFIX`, and rebuilt the three rm `HARDLINE_PATTERNS` on top of
them, adding the `${HOME}` brace form. Kept as plain concatenation so regex
backslashes never land inside an f-string field (Python 3.11 floor).
- `tests/tools/test_hardline_blocklist.py`: added quoted (`"/"`, `'/'`,
`"/etc"`, `"$HOME"`, ...) and brace (`${HOME}`, `"${HOME}"`) cases to the
must-block set, a dedicated `_QUOTED_BRACE_BYPASS` regression parametrization,
no-false-positive guards (`git commit -m "rm -rf /"`), and extended the
yolo-cannot-bypass integration test to cover the quoted/brace forms.
## How to Test
1. Reproduce the bypass on `main`: `detect_hardline_command('rm -rf "/"')`
returns `(False, None)` — the floor lets it through.
2. With this change it returns `(True, "recursive delete of root filesystem")`;
the same holds for `'/'`, `"/etc"`, `"$HOME"`, `${HOME}`, `"${HOME}"`.
3. Run the suite: `scripts/run_tests.sh tests/tools/test_hardline_blocklist.py`
— 125 passed, including the new bypass and no-false-positive cases.
## 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
- [x] I've added tests for my changes (required for bug fixes)
- [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) — pattern-only change, ruff + footgun gate pass
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
Inside httpx AsyncClient response event hooks, response.next_request is
often None even for a genuine redirect, so guards keyed on
`if response.is_redirect and response.next_request` silently never fire.
A public URL that 302s to http://169.254.169.254/ was followed anyway,
defeating the pre-flight is_safe_url() check.
Resolve the redirect target from the Location header (via urljoin, so
relative Locations work too), falling back to next_request only when no
Location is present. Extracted as tools.url_safety.redirect_target_from_response
and wired into every SSRF redirect guard:
- gateway/platforms/base.py (shared image + audio download for all platforms)
- tools/vision_tools.py (two download hooks)
- plugins/platforms/slack/adapter.py
Original fix by @zapabob (PR #35940), which targeted the since-refactored
gateway/platforms/slack.py; reconstructed onto the current shared sites and
widened to the whole bug class.
Review of the salvage found the timeout-message redaction left the more
common failure mode unguarded: when the first websockets.connect(cdp_url)
fails (bad URI / refused / TLS), the raw websockets exception -- which
embeds the full cdp_url incl. ?token= and user:pass@ -- is stashed as
_start_error and re-raised verbatim by start(), and two reconnect
logger.warning sites log the same raw exception.
Add a module-level _redact_cdp_error_text() chokepoint (delegating to
agent.redact.redact_cdp_url) and route all four supervisor egress points
through it:
- start() TimeoutError message (already covered; kept)
- start() _start_error re-raise -> now raises a redacted RuntimeError with
'from None' so no secret leaks via message OR traceback cause chain
- connect-failed and session-dropped reconnect warnings
Guard tests assert the re-raised message is redacted for both token and
userinfo, the raw cause is suppressed, and the helper preserves non-secret
context (host/reason). Verified with a mutation check: reverting to the raw
'raise err' fails the new tests. Correct the redact_cdp_url docstring to
scope its guarantee to direct-URL redaction and point exception callers at
the supervisor helper.
PR #54851 added _sanitize_url_for_logs() and wired it into the three log
sites inside _resolve_cdp_override(). A fourth site was missed:
_create_cdp_session() logs the already-resolved cdp_url unconditionally,
and CDPSupervisor.start() interpolates the raw cdp_url[:80] into the
attach-timeout TimeoutError (which _ensure_cdp_supervisor() logs with %s).
Both leak query-string credentials (e.g. ?token=secret from hosted CDP
providers) into Hermes logs.
Sanitize the URL at both remaining sites. The raw URL is preserved
unmodified in the returned session dict and used for the real connection;
only the logged/error representation is redacted.
Salvaged from #55883.
Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
Salvaged from PR #35130 (the safe subset of jnibarger01's security pass):
- threat_patterns.py: replace unbounded (?:\w+\s+)* filler with bounded
{0,8} + cap scan input at MAX_SCAN_CHARS (64KiB), and bound the .*
runs in the exfil/config-mod patterns. Kills catastrophic backtracking
on adversarial near-misses.
- hermes_state.py: cap FTS5 query length (MAX_FTS5_QUERY_CHARS) and
extract quoted phrases with a linear scan instead of a regex so
pathological quote runs can't induce backtracking.
- acp_adapter/edit_approval.py + agent/tool_dispatch_helpers.py: recognize
'*** Move File: src -> dst' V4A headers so patch-mode edits are
permissioned/traversal-checked (previously only Update/Add/Delete), and
surface a proposal for mode=patch V4A calls (previously replace-only).
Tests: +ReDoS-bound + FTS5-cap + Move-File-target + V4A-approval cases.
The MCP input-schema normalizer in _normalize_mcp_input_schema promotes the
legacy JSON Schema 'definitions' meta-keyword to '$defs' (draft 2019-09+)
so local '$ref' resolution works downstream. The previous walk renamed
*any* key named 'definitions' anywhere in the tree, including inside
'properties' dicts. That turned user-facing parameter names into '$defs',
producing property keys that contain '$', which Anthropic and OpenAI
both reject with HTTP 400 (pattern '^[a-zA-Z0-9_.-]{1,64}$').
Real-world repro: an MCP server that exposes a CI/pipelines tool whose
'definitions' parameter is an array of pipeline-definition IDs. Such a tool
is enough on its own to break every conversation, because the full tools
array is sent on every request.
Fix: when descending into a 'properties' or 'patternProperties' mapping,
iterate property-name -> schema pairs directly, leaving the property names
verbatim. Ordinary JSON Schema semantics resume inside each property's
schema, so a legitimately nested 'definitions' meta-keyword inside a
property's schema is still promoted.
Adds two regression tests:
- test_definitions_as_property_name_is_preserved (the property-name case)
- test_definitions_property_and_meta_keyword_coexist (both forms in one
schema; the property name stays, the meta-keyword promotes)
Follow-up for salvaged PR #35840: current main removed the
use_llm_processing kwarg (LLM summarization dropped) and moved the input
SSRF gate to async_is_safe_url. Adjust the new firecrawl-final-url test
to match.
Two related bugs caused subagent delegation to silently return empty summaries
with 0 tokens when the user configured delegation.provider=bedrock alongside
delegation.base_url=https://bedrock-runtime.<region>.amazonaws.com.
Root cause #1 — misrouting in _resolve_delegation_credentials():
The configured_base_url branch unconditionally forced provider='custom' and
api_mode='chat_completions', only specializing for chatgpt.com, anthropic,
and kimi hosts. Bedrock (and other native-SDK providers) fell through as
'custom' + chat_completions, which then POSTed OpenAI-shaped JSON at
Bedrock's native API. Bedrock rejected the payload and returned nothing,
which looked like an empty LLM response to the child agent.
Fix: when provider is one of {bedrock, vertex, google, google-genai}, skip
the base_url short-circuit and fall through to resolve_runtime_provider(),
which knows how to construct the proper SDK client. base_url can still be
forwarded through that path for regional overrides.
Root cause #2 — '(empty)' sentinel accepted as success:
After N retries of empty LLM responses, run_agent.py emits the literal
string '(empty)' as final_response. _run_single_child then hit
`elif summary:` — '(empty)' is truthy, so status became 'completed' and
the parent surfaced a blank result with no error. Users saw api_calls=4,
tokens=0, duration~0.4s, status=completed.
Fix: treat final_response.strip() == '(empty)' as a failure so the parent
surfaces it instead of silently accepting zero-content 'success'.
Both paths were reproduced in a live Hermes TUI session on us-west-2 Bedrock
(provider=bedrock, model=us.anthropic.claude-sonnet-4-6) and are covered by
new tests in tests/tools/test_delegate.py.
Follow-up to the salvaged #22070. The cron-deny tirith ImportError branch
was unconditionally fail-open; now it honours security.tirith_fail_open:
false by blocking (a cron session has no user to approve), mirroring the
main flow's fail-closed synthesis (#20733).
Adds regression tests: tirith-only content threat blocked in cron-deny,
plus fail-closed/fail-open ImportError behavior.
Text-only Matrix messages sent via the send_message engine (hermes send,
cron deliver: matrix) arrived unencrypted (red padlock) in E2EE rooms.
Media sends already routed through the mautrix adapter and encrypted fine,
but text-only sends took the raw-HTTP standalone_sender_fn path, which
never encrypts.
Route ALL Matrix sends through _send_matrix_via_adapter so text is
encrypted too. The adapter reuses the live gateway's E2EE session when
available (#46310) and falls back to an encryption-aware ephemeral adapter
for standalone/cron contexts. The registry standalone_sender_fn stays
registered for the contract; it is simply no longer reached for Matrix.
Salvaged from PR #20259 onto current main (the original patched the
pre-#41112 _send_matrix branch, which had since moved to the plugin's
standalone path).
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
GNU tools accept unique long-option prefix abbreviations at runtime, so
`chown --recurs root` and `git push --forc` evaded the approval gate's
exact-match `--recursive`/`--force` patterns. Switch those two entries
to prefix matches (--recur[a-z]*, --forc[a-z]*).
The rm/chmod/sed long-flag patterns were left unchanged: every abbreviation
of those is already caught by the sibling short-flag and target patterns
(rm -[^s]*r, base chmod 777, sed -[^s]*i), so prefix-matching them is a
no-op. Only chown (beyond the coincidental case-insensitive r->R catch) and
git push had genuine gaps.
Co-authored-by: Subway2023 <subw3@mail2.sysu.edu.cn>
