From 52a79d99d2b443b04145322402155fd5b172394b Mon Sep 17 00:00:00 2001 From: Societus <93468672+Societus@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:18:05 -0700 Subject: [PATCH] fix(security): TUI approval overlay accepts blind keystrokes, CLI thread-local callback invisible to agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- cli.py | 20 ++++++++++++++++++++ ui-tui/src/app/useInputHandlers.ts | 11 +++++++++++ 2 files changed, 31 insertions(+) diff --git a/cli.py b/cli.py index 05015752a..588988d8c 100644 --- a/cli.py +++ b/cli.py @@ -8370,6 +8370,17 @@ class HermesCLI: def run_agent(): 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 # Prepend pending model switch note so the model knows about the switch _msn = getattr(self, '_pending_model_switch_note', None) @@ -8395,6 +8406,15 @@ class HermesCLI: "failed": True, "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 # process alive when the user closes the terminal tab — SIGHUP diff --git a/ui-tui/src/app/useInputHandlers.ts b/ui-tui/src/app/useInputHandlers.ts index f0e5b3047..25243e992 100644 --- a/ui-tui/src/app/useInputHandlers.ts +++ b/ui-tui/src/app/useInputHandlers.ts @@ -172,6 +172,17 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { const live = getUiState() 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 (key.escape || isCtrl(key, ch, 'c') || ch === 'q') { return patchOverlayState({ pager: null })