Skip to content

fix(security): register shell hooks in TUI gateway and ACP adapter entry points - #41555

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/shell-hooks-tui-acp-registration
Closed

fix(security): register shell hooks in TUI gateway and ACP adapter entry points#41555
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/shell-hooks-tui-acp-registration

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Registers declarative shell hooks from config.yaml in the TUI gateway (desktop app) and ACP adapter (IDE integration) entry points, mirroring the existing registration in cli.py and gateway/run.py. Without this fix, pre_tool_call block hooks are silently ignored when driving the agent from the desktop app or IDE — a security-relevant gap.

Related Issue

Fixes #41457

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/entry.py: Call register_from_config(load_config(), accept_hooks=False) after _install_sidecar_publisher(), wrapped in try/except to never block startup
  • acp_adapter/entry.py: Call register_from_config(load_config(), accept_hooks=False) after _load_env(), same try/except pattern
  • tests/tui_gateway/test_shell_hook_registration.py: 4 tests verifying registration is called and failures don't crash startup in both entry points

How to Test

  1. Add a blocking pre_tool_call shell hook to ~/.hermes/config.yaml
  2. Run hermes hooks doctor — should show all green
  3. Start the desktop app (hermes desktop) or an IDE via ACP
  4. Ask the agent to perform the blocked action — hook should fire and block it
  5. Run pytest tests/tui_gateway/test_shell_hook_registration.py -v — all 4 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: tui_gateway/entry.py, acp_adapter/entry.py (mirroring gateway/run.py:4486-4494 pattern)
  • Blast radius: LOW — adds startup hook registration, no runtime behavior change
  • Related patterns: register_from_config() is idempotent and already called from 3 other entry points

…try points

Shell hooks (pre_tool_call block hooks) were silently ignored in the
desktop app (TUI gateway) and ACP adapter (IDE integration) because
register_from_config() was never called. This created a security gap
where hook-configured tool vetoes were bypassed in those surfaces.

Mirror the gateway/run.py registration pattern in both entry points.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/acp Agent Communication Protocol adapter duplicate This issue or pull request already exists labels Jun 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #41464 — both fix #41457 by calling register_from_config() in the TUI gateway and ACP adapter entry points. (#41464 wires ACP via acp_adapter/session.py, this one via acp_adapter/entry.py.) Same long-standing gap as #13854/#28953. Maintainers should consolidate.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for the flag @alt-glitch. Comparing the two PRs:

The production code changes are equivalent, but this PR includes comprehensive test coverage that #41464 lacks. Keeping open — happy to consolidate if #41464 is preferred by maintainers.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Update: #41464 by @EdderTalmor has been APPROVED by @tonydwb.

Both PRs address the same issue (#41457) with equivalent production code changes. #41464 was opened first and has already received maintainer approval, so closing this PR in favor of #41464.

Thanks for the review @tonydwb — looking forward to seeing #41464 merged.

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/) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists 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

2 participants