Skip to content

fix: register shell hooks in TUI gateway and ACP adapter (issue #41457) - #41464

Open
EdderTalmor wants to merge 1 commit into
NousResearch:mainfrom
EdderTalmor:fix/41457-shell-hooks-tui-acp
Open

fix: register shell hooks in TUI gateway and ACP adapter (issue #41457)#41464
EdderTalmor wants to merge 1 commit into
NousResearch:mainfrom
EdderTalmor:fix/41457-shell-hooks-tui-acp

Conversation

@EdderTalmor

Copy link
Copy Markdown

Summary

Fixes issue #41457 — Shell hooks (the hooks: block in config.yaml) were not being registered in the desktop app (TUI gateway) or the ACP adapter (IDE integration) entry paths. This caused pre_tool_call block hooks to silently do nothing in those surfaces — a security-relevant gap where configured protections were enforced in CLI/gateway but ignored in desktop/IDE.

Root Cause

agent.shell_hooks.register_from_config() was called at startup in:

  • cli.py (line 946)
  • hermes_cli/main.py (line 12714)
  • gateway/run.py (line 4503)

But was not called in:

  • tui_gateway/entry.py (desktop app backend)
  • acp_adapter/session.py (IDE integration / ACP adapter)

Both surfaces build real AIAgent instances and dispatch tools through handle_function_call, which does contain the pre_tool_call block check. The check runs, but the in-process hook registry is empty because register_from_config was never called — so it finds zero hooks and allows everything.

Fix

Added register_from_config(load_config(), accept_hooks=False) calls in both entry points, wrapped in try/except to never block startup. The accept_hooks=False matches the gateway pattern since neither surface has a TTY — consent comes from --accept-hooks, HERMES_ACCEPT_HOOKS, or hooks_auto_accept: true in config.

Changes

  • tui_gateway/entry.py: Added shell hook registration after MCP discovery setup, before gateway.ready event
  • acp_adapter/session.py: Added shell hook registration in _make_agent() after agent creation

Verification

  • All existing shell hook tests pass (tests/agent/test_shell_hooks.py, tests/agent/test_shell_hooks_consent.py)
  • All ACP adapter tests pass (tests/acp_adapter/)
  • All gateway platform reconnect tests pass (tests/gateway/test_platform_reconnect.py)

Related

Closes #41457

…esearch#41457)

Shell hooks (the `hooks:` block in `config.yaml`) were not being registered
in the desktop app (TUI gateway) or the ACP adapter (IDE integration) entry
paths. This caused `pre_tool_call` block hooks to silently do nothing in
those surfaces — a security-relevant gap where configured protections were
enforced in CLI/gateway but ignored in desktop/IDE.

Root cause: `agent.shell_hooks.register_from_config()` was called at startup
in `cli.py`, `hermes_cli/main.py`, and `gateway/run.py`, but not in
`tui_gateway/entry.py` or `acp_adapter/session.py`.

Fix: Add `register_from_config(load_config(), accept_hooks=False)` calls in
both entry points, wrapped in try/except to never block startup. The
`accept_hooks=False` matches the gateway pattern since neither surface has
a TTY — consent comes from `--accept-hooks`, `HERMES_ACCEPT_HOOKS`, or
`hooks_auto_accept: true` in config.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Register shell hooks at TUI gateway startup and in ACP adapter session creation — two coordinated call-sites, no new abstraction needed.

Highlights

  • Both banners explain the consent model (accept_hooks=False -> register_from_config reads env+config) clearly.
  • except Exception: logger.debug(..., exc_info=True) is a non-blocking, fail-open pattern appropriate for optional hook registration.
  • TUI uses load_config() (reads disk); ACP adapter receives config via kwargs — the two call signatures differ, so a shared helper would smuggle in either a config-loader dependency or a passed-by-hand side-effect. The duplication is justified.

No issues.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused security fix. The reported registration gap remains on current main: tui_gateway/entry.py:293-355 reaches gateway.ready without registration, and acp_adapter/session.py:597-645 builds ACP agents without it. This matters because agent/shell_hooks.py:244-280 installs callbacks into the plugin manager and model_tools.py:1181-1200 enforces their block directives.

Problems

  • The PR adds no regression coverage. The changed files are only acp_adapter/session.py and tui_gateway/entry.py; current targeted TUI/ACP tests do not cover shell-hook registration.

Suggested changes

  • Add isolated tests for tui_gateway.entry.main() and SessionManager._make_agent() that verify an approved configured pre_tool_call hook is registered, plus the intended non-fatal failure behavior.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell hooks not registered in desktop (TUI gateway) and ACP adapter — pre_tool_call block hooks silently ignored

4 participants