[security] fix(tui+cli): approval overlays accept blind keystrokes / thread-local callback bypass#13634
Closed
Societus wants to merge 1 commit into
Closed
Conversation
…ead-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 NousResearch#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 NousResearch#13618
|
🚢 please. Massive blockage |
Contributor
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security: Approval Overlay Blind Keystroke Bypass + CLI Thread-Local Callback Invisible to Agent
Discovered while replicating #13618 (TUI approval overlay freezes terminal).
Bug 1: TUI — Blind Keystroke Approval (Security)
Severity: High — dangerous commands can execute without informed user consent.
When the Ink TUI displays an approval overlay (dangerous command detected), the overlay appears visually frozen — arrow keys, number keys, and Enter produce no visible feedback. However, Ink's
EventEmitterdelivers keystrokes to all registereduseInputlisteners, not just the first. TheApprovalPromptcomponent still receives and processes keystrokes even thoughuseInputHandlershas already consumed the event.This means a user pressing keys during the frozen state can:
1→ silently approve the command once2→ approve ALL dangerous commands for the rest of the session (approve_session())3→ write to the permanent allowlist on disk (approve_permanent()+save_permanent_allowlist())4→ deny (the only safe option)The user sees a frozen overlay and has no idea their keystrokes are being processed. Subsequent dangerous commands in the same session (or permanently, if
alwayswas selected) execute without any approval prompt.Root cause:
useInputHandlers(app-level) registers on the sameinputEmitterasApprovalPrompt(component-level). Ink fires ALL listeners. ThereturninuseInputHandlersonly exits that handler — it does not stop propagation.Fix: In
useInputHandlers, whenoverlay.approval || overlay.clarify || overlay.confirmis active, only interceptCtrl+C(for deny/dismiss). All other keystrokes pass through so the overlay renders visual feedback and the user can make an informed selection.Note: This fix makes the overlay responsive but does not change the EventEmitter fan-out behavior. A deeper hardening would be to consolidate overlay input handling into a single handler with explicit routing, but that is a larger refactor.
Bug 2: CLI — Thread-Local Callback Invisible to Agent Thread
Severity: Medium — approval prompt appears but cannot be interacted with, terminal unusable for 60s.
In the legacy CLI (prompt_toolkit),
_callback_tlsintools/terminal_tool.pyisthreading.local().set_approval_callback()is called in the main thread duringrun()initialization (cli.py:~9046), but the agent runs in a background thread (threading.Thread(target=run_agent)). The agent thread calls_get_approval_callback()→ returnsNone(different thread-local slot) → falls back toprompt_dangerous_approval()withstdininput()→ prompt_toolkit owns the terminal → user sees the approval text but cannot respond. Terminal is unusable until 60s timeout expires with default "deny".Fix: Set callbacks inside
run_agent()(the thread target function), matching the pattern already used byacp_adapter/server.pyfor ACP sessions. Clear callbacks toNonein afinallyblock on thread exit to prevent stale references.Files Changed
ui-tui/src/app/useInputHandlers.ts— skip keystroke consumption for approval/clarify/confirm overlayscli.py— set terminal_tool callbacks inside agent thread, clear on exitTesting
tests/cli/test_cli_approval_ui.py)tsc --noEmit)Closes #13618