BaseEnvironment writes shell snapshots and cwd metadata through the process
umask. With a common 022 umask, snapshot files containing exported environment
state landed at mode 0644 even though they can include env-carried credentials
from the parent process.
Set umask 077 only around Hermes metadata writes: the initial snapshot
bootstrap and the post-command snapshot/cwd refresh. User commands still run
under the caller's original umask, while Hermes-owned snapshot and cwd files
are created owner-only.
This intentionally does not copy the source PR's global orphan sweep; deleting
all matching /tmp snapshot files could interfere with concurrent Hermes
processes. The security-critical local disclosure fix is the file mode clamp.
This is salvageable because the source report still identifies a concrete
credential-disclosure path, but the safe subset is smaller than the original
proposal: clamp only the Hermes-owned snapshot writes and leave process-wide
cleanup, user command umask, and concurrent sessions alone.
Salvages source PR: https://github.com/NousResearch/hermes-agent/pull/20056
Related issue: https://github.com/NousResearch/hermes-agent/issues/48441
Co-authored-by: Andrew Homeyer <andrew@hndl.app>
Same bug class as skills_tool: module-level SKILLS_DIR pinned at import
under the launch HERMES_HOME makes skill_manage() write/edit against the
wrong profile in long-lived multi-profile runtimes. Apply the same
_skills_dir() call-time resolution (honoring explicit test patches of
SKILLS_DIR) to _containing_skills_root, _resolve_skill_dir,
_find_skill_in_other_profiles, and create-result path reporting.
Refs #40677
Port from anomalyco/opencode#31877: JSON Schema type arrays like
["number","string"] (common in MCP tool schemas) were collapsed to the
first non-null type, silently dropping every other branch. Several
tool-call backends reject the array form outright — llama.cpp's grammar
generator and Gemini via OpenAI-compatible transports (e.g. GitHub
Copilot proxying to Gemini) 400 on it.
_sanitize_node now mirrors @ai-sdk/google: a single non-null type stays
type:X (+nullable if null was present), multiple non-null types become
an anyOf of single-type schemas so no branch is lost, and an all-null
array becomes type:null. Single-null collapse is unchanged.
Verified nested (object props, array items) survive the full sanitize
pipeline — combinator stripping is top-level-only and nullable-union
collapse only fires on single-survivor unions, so multi-type anyOf is
left intact.
Port from openclaw/openclaw#91950: normalize LLM-generated URLs like 'https:// docs.example' before web tool safety checks while preserving path and query encoding semantics.
Port from nearai/ironclaw#4547: treat a JSON null memory target as omitted so strict providers that fill optional fields with null use the documented default target instead of failing validation.
A user-approved terminal/execute_code command could be SIGINT-killed
(exit 130 + "[Command interrupted]") by a stale interrupt bit that landed
on the execution thread during the blocking approval-wait, while the
result still carried the "...approved by the user." note. The terminal
tool runs sequentially inline on the execution thread, and nothing
cleared or re-checked the bit between approval-grant and env.execute.
Clear the current thread's interrupt bit once before an approved command
spawns its child (terminal foreground; execute_code local + remote), and
enrich the note to "...approved by the user, then interrupted." on a
genuine post-start interrupt instead of implying success. A genuine
interrupt arriving after execution starts (or during a retry backoff)
still SIGINTs the command; non-approved commands keep current behavior.
Adds regression tests covering stale-bit-clears, genuine-interrupt-still-
kills, the retry-backoff window, natural-exit-130 (not mislabeled), and
execute_code local + remote.
When a bundled web provider (firecrawl, tavily, exa, ...) is listed in
plugins.disabled, its provider never registers and the web_search/
web_extract dispatchers emitted the misleading "No web extract provider
configured. Set web.extract_backend to ..." — even though the backend was
configured correctly. The real fix is to re-enable the plugin.
- web_tools.py + web_search_registry.py: when the configured backend names
a disabled bundled web plugin, both dispatchers now point the user at the
actual cause (re-enable the plugin) instead of a wrong config hint.
- plugins_cmd.py cmd_enable: enabling by canonical key now also clears the
manifest-name alias (web-firecrawl) from plugins.disabled, so the
suggested command actually re-enables the plugin ('explicit disable wins'
matches on the name too).
- plugins_cmd.py cmd_toggle / _run_composite_ui / _run_composite_fallback:
the interactive 'hermes plugins' menu now persists the canonical key
(web/firecrawl), never the bare manifest name — the drift that put the
offending entry in plugins.disabled in the first place.
Follow-up to #59518 (which fixed web credential resolution, a different
cause). Fixes the disabled-plugin symptom reported after that PR.
The Firecrawl provider used os.getenv() to read FIRECRAWL_API_KEY and
FIRECRAWL_API_URL, which only checks the process environment. When
values are supplied through Hermes's ~/.hermes/.env config mechanism
(via hermes_cli.config.get_env_value), they are not guaranteed to be
present in os.environ for every gateway/tool execution path.
Switch to get_env_value() which checks both os.environ and the .env
file, matching the pattern used by other providers (nous_subscription,
setup, discord adapter).
Fixes#40190
_sync_back_once defers a SIGINT that lands mid-sync, then re-delivers it once the
sync completes so the user's Ctrl+C isn't lost. It did so with
os.kill(os.getpid(), signal.SIGINT). That is not graceful on Windows: os.kill
only treats CTRL_C_EVENT(0)/CTRL_BREAK_EVENT(1) as console events; any other
value (SIGINT == 2) routes to TerminateProcess(sig), so a Ctrl+C during a
remote-backend (ssh/daytona/modal) sync-back hard-kills the whole CLI session
(exit code 2) on Windows instead of raising KeyboardInterrupt.
Use signal.raise_signal(signal.SIGINT) (3.8+), which invokes the restored
handler through C raise() on every platform. Verified on Windows: raise_signal
runs the handler (graceful) while os.kill(getpid, SIGINT) TerminateProcess-es
the process. Adds a cross-platform regression test that runs on Windows too (it
stubs the locked sync body, so unlike test_file_sync_back.py it needs no fcntl).
The Skills Hub 'Browse Hub' landing page and index-backed search render
empty on fresh deployments (e.g. Fly.io VPS agents) with no stale cache.
Root cause: the centralized index at /docs/api/skills-index.json is a
large body (~34MB, tens of MB compressed) served with Content-Encoding:
br. httpx's streaming Brotli decoder — backed by brotlicffi 1.2.0.1,
which is pinned so aiohttp can decode Discord attachments — trips over
its own output_buffer_limit on a payload this size and raises:
DecodingError("brotli: decoder process called with data when
'can_accept_more_data()' is False")
_load_hermes_index() catches that (DecodingError is an httpx.HTTPError
subclass) and silently falls back to the on-disk cache. On a fresh box
that cache never existed, so HermesIndexSource.is_available is False,
the index contributes 0 skills, and the hub landing page — which is
built solely from an empty-query index search — is blank. Existing
installs only appear to work because they serve a (possibly weeks-)stale
cached index instead.
Fix: request 'gzip, deflate' on the index fetch so httpx never
negotiates the broken Brotli path, and retry once with 'identity' if a
DecodingError still occurs (defends against a proxy that ignores the
header). Falls through to the stale cache only when both attempts fail.
Verified on a live staging VPS agent: index_available flips False->True
and the featured landing list repopulates from 0 to 12.
Also un-freezes already-deployed images: skills added after an image was
built (e.g. the 'unbroker' optional skill) become reachable again via
the index, which is the whole point of the centralized catalog.
register_mcp_servers now nudges cached entries whose session is None
via _signal_reconnect, so a new agent session recovers a parked server
immediately instead of waiting up to _PARKED_RETRY_INTERVAL for the
next self-probe (#50170). Gate-check idea credit: @izumi0uu (#50184),
@LeonSGP43 (#37772), @Tranquil-Flow (#37899).
_wait_for_server_session_ready used a time.monotonic deadline; the
circuit-breaker tests freeze monotonic, turning the loop into an
infinite spin (300s SIGKILL in CI-parity runs). Bound by iteration
count instead.
Four independent pre-request stalls sat on the critical path between
prompt submission and the first streamed token, measured with cProfile
against a live process:
1. Discord capability detection (~2.0s, worst 5s): get_tool_definitions
-> _get_dynamic_schema made a BLOCKING https call to discord.com
inside AIAgent.__init__ for any user with DISCORD_BOT_TOKEN set, on
every platform, every cold process. Now non-blocking: memory cache ->
24h disk cache -> permissive default + one background detection that
seeds the disk cache for the next process. The permissive default is
pinned per-process so tool schemas never flip mid-conversation
(prompt-cache safety); it mirrors the existing detection-failure
fallback (all actions exposed, 403s enriched at call time).
2. Ollama /api/show probe (~0.3s): get_model_context_length step 5e
POSTed to <base_url>/api/show for KNOWN providers (openrouter etc.),
got a 404, and never cached the miss - so every fresh process paid a
full HTTP round-trip. Known non-Ollama providers now skip the probe;
local/custom/unknown endpoints keep the exact previous behavior.
3. env_probe subprocess sweep (~0.5s): the Python-toolchain probe ran
4-8 subprocess calls inside the FIRST system prompt build. Now warmed
off-thread during agent init; the prompt build hits the cache (same
lock, so a mid-flight warm just joins instead of recomputing).
4. tools.mcp_tool import (~0.4s): the between-turns MCP refresh in
build_turn_context imported the whole mcp package even with zero MCP
servers configured. MCP tools can only exist if tools.mcp_tool was
already imported (discovery/reload paths), so gate the import on
sys.modules membership - no behavior change for MCP users.
CLI additionally pre-imports run_agent + openai off-thread during the
idle banner window (same pattern as the /model picker prewarm), hiding
the remaining ~1.5s of module imports while the user types. Fixes 1-4
apply to every interaction layer (CLI, gateway, TUI, desktop, cron).
Measured cold first turn (submit -> request dispatched, openrouter,
discord token set): 4.3s before -> 0.9s after CLI prewarm (~80%); the
agent-side non-import cost drops 2.9s -> 0.36s (init) + 0.27s (turn
prologue).
Some MCP servers (e.g. Spring Boot apps with a React SPA) serve their
frontend on any unmatched GET route. The MCP endpoint works perfectly
via POST (JSON-RPC), but a GET to /mcp falls through to the SPA
controller and returns text/html. Hermes's preflight content-type probe
sees HTML instead of application/json or text/event-stream and refuses
to connect.
This adds a per-server config option that
bypasses the content-type probe, letting the SDK connect directly via
POST where it works fine.
```yaml
mcp_servers:
stirling-pdf:
url: http://localhost:8090/mcp
headers:
X-API-KEY: <key>
skip_preflight: true
```
Related: #52460 (OAuth redirect preflight), #51600 (skip probe on mcp add),
#40366 (skip probe on reconnect — already merged).
Some MCP servers (e.g. DocuSeal) serve their web UI on HEAD/GET but
speak Streamable HTTP only via POST. The preflight probe now tries a
lightweight JSON-RPC `initialize` POST before rejecting endpoints
whose HEAD/GET returns a non-MCP content type (e.g. `text/html`).
If the POST returns `application/json` or `text/event-stream` with a
2xx status, the endpoint is accepted. Otherwise the original rejection
behaviour is preserved.
Adds 5 new test cases covering the POST probe path:
- POST rescues HTML HEAD with JSON response
- POST rescues HTML HEAD with event-stream response
- POST still rejects when it also returns HTML
- POST still rejects on non-2xx status
- POST not attempted when HEAD already returns valid MCP content type
Parking deregisters the server's tools, which removes the only paths
that could ever set _reconnect_event (circuit-breaker half-open probe
and _signal_reconnect both live inside registered tool handlers). A
parked server was therefore unrevivable short of a manual /mcp reload —
the park comment's promised breaker wake could never fire.
Make the parked wait a timed wait: every _PARKED_RETRY_INTERVAL (300s)
the run task wakes and attempts one revival probe, re-parking on
failure instead of burning the full 5-retry budget each cycle. Explicit
reconnect requests still wake it immediately. Idea credit: @Hellbayne
(PR #38881, earliest never-abandon proposal), reconciled with the
park design from #53599.
_discover_tools only filled self._tools; registry registration happened
only in _discover_and_register_server (initial start) and _refresh_tools.
After parking deregistered a server's tools, a revival rebuilt the
transport but published zero tools — a phantom recovery.
Register freshly discovered tools whenever _ready is set and the
registry entry list is empty. Extracted from PR #54139 by @nicha16
(the remainder of that PR reverses the park design and is not taken).
The local retries variable in MCPServerTask.run() accumulated across
transient disconnections — each transport exception incremented it, but
only clean transport returns (auth recovery / manual refresh) or
park-wake reset it. Five transient blips over a long-uptime gateway
would permanently park the MCP server.
Promote retries to instance attribute _reconnect_retries and reset it
at all 4 session-establishment sites in _run_stdio / _run_http, so only
consecutive failures without successful reconnection count toward the
parking budget.
Fixes#57604
Adds approvals.deny to config.yaml — a list of fnmatch globs matched
against terminal commands. A match blocks unconditionally, BEFORE the
--yolo / /yolo / approvals.mode=off bypass, making it the user-editable
counterpart to the code-shipped hardline blocklist.
- Checked in both command gates (check_dangerous_command and
check_all_command_guards), after the hardline floor and sudo-stdin
guard, before the yolo bypass and permanent allowlist.
- Matching runs over the same normalized/deobfuscated command variants
as the dangerous-pattern detector, case-insensitive.
- Opt-in: empty/absent list is a no-op; behavior unchanged.
Supersedes the trust-engine approach from #21500 with a minimal
config-native design: the only capability the existing stack lacked
was deny-that-beats-yolo. Allow already exists (command_allowlist),
ask already exists (session approvals).
PR #58889 fixed the CLI-fallback transport; review of that fix found the
same leak class at four sibling spawn sites of the third-party cua-driver
binary:
- _resolve_mcp_invocation (cua-driver manifest): no env= at all — full
parent environment inherited
- cua_driver_update_check (check-update --json): telemetry env but no
secret sanitization
- doctor._drive_health_report (<binary> mcp Popen): telemetry env only
- permissions._run (every macOS/Linux permission probe): telemetry env only
All now route through _sanitize_subprocess_env(cua_driver_child_env()),
matching the sanctioned MCP spawn and the #53503/#55709/#58889 strip-by-
default policy for non-terminal spawns. Sanitization degrades gracefully
(falls back to the telemetry env) so doctor/permission probes never break
on an import error.
4 regression tests covering each site.
When a single line exceeds the entire char budget, its tail is
unreachable via offset pagination (offsets are line-granular). Tell
the model so it doesn't assume it saw the full line.
read_file previously hard-rejected any read whose formatted output exceeded
the ~100K char safety limit, returning an error with zero content. A file
with few but very long lines (logs, wide CSV rows, minified data) sails past
the line-count limit and then trips the char guard, so the model gets nothing
and must guess a smaller limit — wasting a full round-trip.
Now the read is trimmed to the last complete line that fits the budget and
returns the partial content plus truncated_by="bytes" and a next_offset, so
the model paginates forward instead of starting over. A single line larger
than the whole budget is clamped on a code-point boundary (never empty) and
the cursor still advances. Applies at both read paths (normal + extracted
documents).
Adapted from IronClaw's Rust dual line/byte cap to hermes's Python tool-layer
char guard, which is the single uniform chokepoint over the gutter-rendered
content for every backend.
Follow-up to the salvaged toggle commit:
- file_tools.py / code_execution_tool.py: carry docker_network in their
container_config dicts so those environment-creation paths honor the
lockdown instead of silently defaulting back to bridge (the probe/exec
asymmetry class reported on #46358).
- docker.py: cross-process reuse now inspects HostConfig.NetworkMode when
docker_network=false and removes a mismatched (networked) container
before starting a fresh air-gapped one. Fails closed when inspect fails.
Default-network config never churns containers, so operators using
docker_extra_args --network=none are unaffected.
- tests: AST invariant that every container_config site carrying
docker_run_as_host_user also carries docker_network, plus three reuse
guard tests (reject bridge under lockdown / keep matching none /
no inspect when network enabled).
- docs: configuration.md gains terminal.docker_network + env var row.
Port from qwibitai/nanoclaw#2713: expose Hermes' existing Docker network isolation primitive through terminal config so operators can opt out of container egress.
_CuaDriverSession._call_tool_via_cli() (the EAGAIN/silent-empty MCP
fallback transport) invokes `cua-driver call <tool> <json>` via
subprocess.run() with no env= argument, so the third-party cua-driver
binary inherits the full, unsanitized parent environment. The primary
MCP spawn site (_lifecycle_coro) already applies
_sanitize_subprocess_env(cua_driver_child_env()) before opening the
stdio client, per the same policy #53503/#55709 established for other
subprocess spawn points — this fallback path, added alongside the
EAGAIN/silent-empty-capture hardening, missed it.
Fix: apply the same env=_sanitize_subprocess_env(cua_driver_child_env())
to the subprocess.run() call in _call_tool_via_cli(), mirroring the
sanctioned spawn site exactly (telemetry policy applied first, then
Hermes-managed secrets filtered).
Port from openai/codex#21069 ("Spill large hook outputs from context").
Both shell hooks and Python plugins can return {"context": "..."} from
pre_llm_call, which gets appended to the current turn's user message on
every subsequent API call. A plugin that emits a large blob inflates
every turn and blows out the prompt cache prefix.
- tools/hook_output_spill.py: shared helper that writes oversized
context to $HERMES_HOME/hook_outputs/<session_id>/<uuid>.txt and
returns a head/tail preview plus the saved path. Never raises.
- agent/turn_context.py: apply the cap at the pre_llm_call aggregation
site (moved here from run_agent.py since the original PR), covering
both Python plugins and shell hooks.
- agent/shell_hooks.py: reserve output_spill as a sub-key under hooks:
so the config block doesn't emit unknown-hook-event warnings.
- Docs: document the cap + config in build-a-hermes-plugin.md.
Config (behaviour-preserving when absent):
hooks.output_spill: enabled/max_chars/preview_head/preview_tail/directory
Tests: 14 unit tests; shell_hooks (56) and plugins (100) suites green.
E2E validated with isolated HERMES_HOME (spill, passthrough, traversal
sanitisation, reserved-key skip).
ruff check --fix --select F541 . on current main. Pure prefix removals;
adjacent-string concatenations keep the f only on interpolating fragments.
No string content or live placeholder altered.
Port from anomalyco/opencode#33533. Native MCP tools now register as
mcp__<server>__<tool> (double-underscore delimiter) instead of
mcp_<server>_<tool>, aligning with the convention used by Claude Code,
Codex, and OpenCode.
The double-underscore delimiter disambiguates the server/tool boundary
even when either component contains underscores (the single-underscore
form was ambiguous, which is why is_mcp_tool_parallel_safe already had to
track provenance in a side-map). It also unifies native registration with
the Anthropic-OAuth wire form (_MCP_TOOL_PREFIX = 'mcp__'), so the
single->double promotion that path performed is now a no-op for native
tools while still handling legacy replayed names.
- tools/mcp_tool.py: add MCP_TOOL_NAME_PREFIX + mcp_prefixed_tool_name()
helper; route _convert_mcp_schema, utility schemas, refresh stale-set,
and the parallel-safe prefix gate through it
- agent/transports/codex_event_projector.py: mirror convention in the
deterministic call_id input for MCP server-executed tool calls
- tests: update produced-name assertions to the new convention
The SOM/AX element list dropped labels for two extremely common cua-driver
render forms, leaving the model unable to target elements by name:
- [79] AXButton (Dark) -> parenthesised label
- [4] AXStaticText = "Wi-Fi" -> = "value" form
- [92] AXPopUpButton = "Automatic" -> = "value" form
The old regex only matched quoted "label" and id=Label, so System Settings
buttons/text/popups all surfaced with empty labels. That's why selecting the
macOS Appearance 'Dark' button by element index required guessing — the
labels weren't available to aim with.
Fix: extend _ELEMENT_LINE_RE to capture all four label forms (= "value",
"quoted", (parenthesised), id=Label), skipping a pure-digit (N) order number
in favour of the id= label. Verified live against System Settings: the
Appearance buttons now surface as Auto/Light/Dark.
Adds a regression test covering all label forms. Full suite: 84 passed.
The first fix handled the EAGAIN McpError path. But the persistent MCP
session (long-running gateway/desktop worker) has a second failure mode:
list_windows or get_window_state 'succeed' over MCP yet return a
degenerate/empty payload (no windows, or no screenshot + blank tree)
WITHOUT raising — typically when the bridge reconnected mid-call and
dropped the heavy response. That surfaced to the model as a silent 0x0
capture with no error and no fallback firing (0.00s empty return).
Fix: detect empty results in capture() and re-fetch over the CLI
transport before giving up:
- empty list_windows -> CLI re-fetch the window list
- empty get_window_state (som/ax) -> CLI re-fetch the AX tree + screenshot
- empty screenshot (vision) -> CLI re-fetch get_window_state for the PNG
Adds 2 regression tests. Full suite: 83 passed.
The cua-driver MCP stdio bridge intermittently (and on some machines
persistently) fails to forward heavier calls like get_window_state to
the daemon with POSIX EAGAIN — 'daemon transport error forwarding
get_window_state: Resource temporarily unavailable (os error 35)'.
The wrapper surfaced this as an empty 0x0 capture, so computer_use
returned blank screenshots even though the display, permissions, and
the daemon were all healthy (the direct 'cua-driver call' CLI path
worked fine throughout).
Fix: when the MCP path raises the transient/transport error, fall back
to the 'cua-driver call' subprocess transport, which talks to the
daemon over a different socket. The CLI fallback routes get_window_state
screenshots to a temp file via screenshot_out_file (tiny JSON response
instead of a multi-MB base64 blob that congests the socket), reads the
PNG back, retries with backoff, and remaps the JSON into the same
{data, images, structuredContent, isError} shape the MCP path produces
so capture()/_action() are transport-agnostic.
Adds _is_transient_daemon_error() classifier and 3 regression tests.
Verified live: captures that returned 0x0 now return full
1567x905 screenshots with the AX element tree.
The 'never reached ready' error (issue #57025) was undiagnosable — doctor
and MCP test pass while the wrapper times out, with no hint where startup
stalled. Track a phase marker through _lifecycle_coro (binary-check →
manifest-discovery → mcp-initialize → capability-discovery → ready) and
include it in the timeout RuntimeError plus a pointer to doctor and the
agent.log phase timings.
Complements the 15s→30s bump + success-path phase timing log from #58760.
On Windows, the cua-driver MCP session initialization can exceed the 15s
timeout due to manifest subprocess discovery + MCP transport setup.
This makes the computer_use tool permanently unavailable even though
hermes computer-use doctor and hermes mcp test both pass.
- Increase _ready_event.wait timeout from 15s to 30s
- Add startup timing instrumentation (manifest + mcp_init durations)
- Log timing at INFO level for diagnosability
Fixes#57025
Follow-up to #56236: the broadened root token /[/.]*\** treats any run of
dots after the root slash as a collapse spelling, so a literal root-level
directory named '...' (rm -rf /...) was unconditionally hardline-blocked
with no approval path. Tighten the token to /(?:(?:\.\.?)?/)*(?:\.\.?)?\**
so each inter-slash segment must be exactly '.' or '..' — all real collapse
spellings (//, /., /./, /.., //*, ///, /../..) stay on the hardline floor
while literal dot-run dirs fall through to the softer DANGEROUS_PATTERNS
rules like every other real path.
* feat(approvals): /deny <reason> relays denial reason to the agent
Port from qwibitai/nanoclaw#2832 (reject with reason).
Gateway /deny now accepts an optional trailing reason (/deny <reason>
or /deny all <reason>). The reason rides on the per-session approval
entry through resolve_gateway_approval -> _await_gateway_decision and is
appended to the BLOCKED tool result the agent receives, so a declined
agent can adapt instead of only hearing 'denied'.
Adapted to hermes-agent's synchronous single-command /deny model: no DB
state, no second-message capture step, no migration. Reason is capped at
280 chars and threaded through both the terminal-command guard and the
execute_code guard. Plain /deny and the approve paths are unchanged.
- tools/approval.py: _ApprovalEntry.reason; resolve_gateway_approval gains
optional reason; _await_gateway_decision returns it; both gateway BLOCKED
messages include it
- gateway/slash_commands.py: parse leading 'all' + trailing reason
- locales/en.yaml: deny.denied_reason_{singular,plural}
- hermes_cli/commands.py: /deny args_hint '[all] [reason]'
- tests: 3 new (with-reason, all+reason, plain-deny regression)
* fix(ci): localize deny-reason keys across all locales + update interrupt-path assertions
CI surfaced two enforced invariants broken by the deny-with-reason change:
- test_i18n catalog-parity requires every locale to carry the same keys as
en.yaml with matching placeholders. Added deny.denied_reason_singular/plural
(with {count}/{reason}) to all 15 non-English locales.
- test_approval_interrupt asserts the exact dict from _await_gateway_decision,
which now carries a 'reason' key (None on the interrupt/timeout paths).
Port from anomalyco/opencode#34529: MCP servers can emit
notifications/message logging notifications (RFC 5424 levels), but the
MCP SDK's default logging_callback silently discards them — server-side
warnings/errors during tool calls were invisible.
- tools/mcp_tool.py: pass a logging_callback to every ClientSession
(stdio, SSE, streamable HTTP old+new API paths via the shared
sampling_kwargs sites), mapping the 8 MCP log levels onto Python
logging levels and tagging entries with [server/logger] origin.
- JSON-serialize non-string payloads, cap at 2000 chars so a chatty
server can't flood agent.log, never raise from the handler.
- Gated on SDK support (_check_logging_callback_support) mirroring the
existing message_handler gate for old SDK versions.
- tests/tools/test_mcp_server_log_notifications.py: 10 tests covering
level mapping, origin tagging, JSON payloads, truncation, and the
never-raise contract.
video_analyze_tool's local-path branch read raw bytes via
_detect_video_mime_type (extension-only, no magic-byte check) with no
call to agent.file_safety.raise_if_read_blocked, unlike the image-gen
and video-gen provider plugins that already route local inputs through
that shared chokepoint (#57698). A model could point video_url at a
credential store (e.g. .env, auth.json) renamed or symlinked to a
video-like extension and have its raw bytes base64-encoded and sent to
the vision provider.
vision_analyze_tool and its native fast path (_vision_analyze_native)
had the same gap in their local-file branches; they were only
incidentally protected by the image magic-byte sniff rejecting
non-image content, not by the intended read guard.
Add raise_if_read_blocked() to all three local-file branches, mirroring
the existing plugins/image_gen and plugins/video_gen call sites.
Extend the pre_tool_call plugin hook return contract with a new directive:
{"action": "approve", "message": "why this needs human confirmation"}
Previously a pre_tool_call hook could only veto a tool call (action: block)
or allow it silently. It could not escalate to the existing human-approval
flow. This unlocks user-defined runtime approval rules on ANY tool (HTTP
writes, file writes to sensitive paths, email sends), enforced at runtime —
resolving #51221 as a pure plugin, with no core approval.py rule schema.
Mechanism:
- get_pre_tool_call_directive() returns (action, message) for block|approve;
get_pre_tool_call_block_message() kept as a block-only back-compat shim.
- resolve_pre_tool_block() is the single dispatch-site chokepoint: fetches
the directive and, for approve, invokes the human gate; fail-closed to a
block on denial, timeout, or gate exception. ALL FOUR tool-dispatch sites
now call it: tool_executor (concurrent + sequential), agent_runtime_helpers,
and model_tools.handle_function_call.
- request_tool_approval() escalates via the SAME machinery as Tier-2
dangerous commands: session/permanent allowlist, prompt_dangerous_approval
(CLI) / submit_pending (gateway), [o]nce/[s]ession/[a]lways/[d]eny,
timeout fail-closed, approvals.cron_mode for cron contexts.
Architecture: extracted the shared decision core into _run_approval_gate(),
called by BOTH check_dangerous_command() and request_tool_approval() so the
fail-closed / cron / gateway / yolo / persist policy lives in ONE place and
cannot drift. Fixed a latent divergence — the plugin path now honors --yolo.
Approval grain: [a]lways is keyed on tool_name + a hash of the reason (an
explicit plugin rule_key overrides), so distinct reasons on the same tool
persist independently instead of one 'always' blanketing the whole tool.
Non-interactive: cron honors approvals.cron_mode (parity with commands); any
other non-interactive non-gateway context fails CLOSED for the plugin path
(the command path keeps its historical fail-open default, unchanged).
No new config schema, no new env vars, no new hook events.
The container exec-read piped the whole file through base64 with no size
guard — the 50MB cap was only enforced host-side AFTER the full payload
had already streamed into host memory. A prompt-injected read of a huge
container file (or /dev/zero) could balloon the gateway process.
head -c (cap+1) bounds the read inside the sandbox; the +1 byte lets the
host distinguish at-cap from over-cap and reject with SourceTooLarge.
Input redirect replaces 'base64 --' (no argv exposure at all for
leading-dash paths). Docker integration tests re-verified live.
The salvaged #52688 rasterizer shell-out predates the TUI subprocess
stdin= guard; a rasterizer that prompts on stdin could hang the tool
under prompt_toolkit. DEVNULL it.
