mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(security): TUI approval overlay accepts blind keystrokes, CLI thread-local callback invisible to agent
Two bugs that allow dangerous commands to execute without informed user consent. TUI (Ink): useInputHandlers consumes the isBlocked return path, but Ink's EventEmitter delivers keystrokes to ALL registered useInput listeners. The ApprovalPrompt component receives arrow keys, number keys, and Enter even though the overlay appears frozen. The user sees no visual feedback, but keystrokes are processed — allowing blind approval, session-wide auto-approve (choice "session"), or permanent allowlist writes (choice "always") without the user knowing. Discovered while replicating #13618 (TUI approval overlay freezes terminal). Fix: in useInputHandlers, when overlay.approval/clarify/confirm is active, only intercept Ctrl+C. All other keys pass through. This makes the overlay visually responsive so the user can see what they are selecting. CLI (prompt_toolkit): _callback_tls in terminal_tool.py is threading.local(). set_approval_callback() is called in the main thread during run(), but the agent executes in a background thread. _get_approval_callback() returns None in the agent thread, falling back to stdin input() which prompt_toolkit blocks. The user sees the approval text but cannot respond — the terminal is unusable until the 60s timeout expires with a default "deny". Fix: set callbacks inside run_agent() (the thread target), matching the pattern already used by acp_adapter/server.py. Clear on thread exit to avoid stale references. Closes #13618
This commit is contained in:
parent
204f435b48
commit
52a79d99d2
2 changed files with 31 additions and 0 deletions
20
cli.py
20
cli.py
|
|
@ -8370,6 +8370,17 @@ class HermesCLI:
|
||||||
|
|
||||||
def run_agent():
|
def run_agent():
|
||||||
nonlocal result
|
nonlocal result
|
||||||
|
# Set callbacks inside the agent thread so thread-local storage
|
||||||
|
# in terminal_tool is populated for this thread. The main thread
|
||||||
|
# registration (run() line ~9046) is invisible here because
|
||||||
|
# _callback_tls is threading.local(). Matches the pattern used
|
||||||
|
# by acp_adapter/server.py for ACP sessions.
|
||||||
|
set_sudo_password_callback(self._sudo_password_callback)
|
||||||
|
set_approval_callback(self._approval_callback)
|
||||||
|
try:
|
||||||
|
set_secret_capture_callback(self._secret_capture_callback)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
agent_message = _voice_prefix + message if _voice_prefix else message
|
agent_message = _voice_prefix + message if _voice_prefix else message
|
||||||
# Prepend pending model switch note so the model knows about the switch
|
# Prepend pending model switch note so the model knows about the switch
|
||||||
_msn = getattr(self, '_pending_model_switch_note', None)
|
_msn = getattr(self, '_pending_model_switch_note', None)
|
||||||
|
|
@ -8395,6 +8406,15 @@ class HermesCLI:
|
||||||
"failed": True,
|
"failed": True,
|
||||||
"error": _summary,
|
"error": _summary,
|
||||||
}
|
}
|
||||||
|
finally:
|
||||||
|
# Clear thread-local callbacks so a reused thread doesn't
|
||||||
|
# hold stale references to a disposed CLI instance.
|
||||||
|
try:
|
||||||
|
set_sudo_password_callback(None)
|
||||||
|
set_approval_callback(None)
|
||||||
|
set_secret_capture_callback(None)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
# Start agent in background thread (daemon so it cannot keep the
|
# Start agent in background thread (daemon so it cannot keep the
|
||||||
# process alive when the user closes the terminal tab — SIGHUP
|
# process alive when the user closes the terminal tab — SIGHUP
|
||||||
|
|
|
||||||
|
|
@ -172,6 +172,17 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult {
|
||||||
const live = getUiState()
|
const live = getUiState()
|
||||||
|
|
||||||
if (isBlocked) {
|
if (isBlocked) {
|
||||||
|
// When approval/clarify/confirm overlays are active, their own useInput
|
||||||
|
// handlers must receive keystrokes (arrow keys, numbers, Enter). Only
|
||||||
|
// intercept Ctrl+C here so the user can deny/dismiss — all other keys
|
||||||
|
// fall through to the component-level handlers.
|
||||||
|
if (overlay.approval || overlay.clarify || overlay.confirm) {
|
||||||
|
if (isCtrl(key, ch, 'c')) {
|
||||||
|
cancelOverlayFromCtrlC()
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if (overlay.pager) {
|
if (overlay.pager) {
|
||||||
if (key.escape || isCtrl(key, ch, 'c') || ch === 'q') {
|
if (key.escape || isCtrl(key, ch, 'c') || ch === 'q') {
|
||||||
return patchOverlayState({ pager: null })
|
return patchOverlayState({ pager: null })
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue