Skip to content

fix(runner): Harden centralized keyboard hook against stuck keys#8

Closed
crutkas wants to merge 1 commit into
mainfrom
fix-stuck-keys/b-centralized-hook
Closed

fix(runner): Harden centralized keyboard hook against stuck keys#8
crutkas wants to merge 1 commit into
mainfrom
fix-stuck-keys/b-centralized-hook

Conversation

@crutkas

@crutkas crutkas commented Apr 13, 2026

Copy link
Copy Markdown
Owner

Summary

Harden the centralized keyboard hook (centralized_kb_hook.cpp) against stuck keys, ghost activations, and cross-hook interference.

Changes

B1: Tag dummy 0xFF SendInput with dwExtraInfo

The dummy keystroke injected after hotkey actions had no dwExtraInfo tag, causing other hooks (KBM, PowerAccent) to process it as real input. Now tagged with CENTRALIZED_KEYBOARD_HOOK_DONT_TRIGGER_FLAG.

B2: Add LLKHF_INJECTED early-out

Skip all injected events not tagged by PowerToys. Prevents On-Screen Keyboard, AutoHotkey, KVM switches, and other macro tools from driving the hotkey/pressed-key state machines.

B3: Revalidate key state in PressedKeyTimerProc

Check GetAsyncKeyState before firing the timer action. Prevents ghost activations when the user releases the key before the timer fires.

B4: Harden Stop()

Kill all pending pressed-key timers and reset vkCodePressed before unhooking. Prevents stale state after module disable/re-enable.

B5: Fix vkCodePressed race conditions

  • Made vkCodePressed an std::atomic<DWORD>
  • Hoisted pressedKeyMutex to cover the entire pressed-key block (TOCTOU fix)
  • Replaced magic 0x100 with VK_DISABLED

How to Reproduce

  1. Set a PowerToys hotkey (e.g., Win+Shift+C for Color Picker)
  2. Press the hotkey 50+ times rapidly
  3. Open Windows On-Screen Keyboard and press hotkey combinations
  4. Before fix: Ghost activations, modifier keys stuck, OSK input triggers hotkeys

How to Verify

  1. Rapid hotkey presses — no ghost activations, no stuck modifiers
  2. OSK input — does not trigger PowerToys hotkeys
  3. Long-press actions — timer does not fire after key is released early
  4. Module disable/re-enable — no stale state

PR Checklist

  • Builds clean on x64 Release
  • All 95 KBM unit tests pass

Comment thread src/runner/centralized_kb_hook.cpp Fixed
@github-actions

This comment has been minimized.

@yeelam-gordon

Copy link
Copy Markdown

Quick check on the new injection filter (centralized_kb_hook.cpp ~L103-107):

if ((keyPressInfo.flags & LLKHF_INJECTED) && keyPressInfo.dwExtraInfo == 0)
{
    return CallNextHookEx(hHook, nCode, wParam, lParam);
}

This skips any injected event that does not carry a non-zero dwExtraInfo. Intent is right (keep external macros from triggering PT), but the blast radius worth verifying before merge:

  • On-Screen Keyboard (osk.exe) does it set dwExtraInfo? On Win11 it generally does not.
  • Eye-control / switch-input / Narrator-driven keys same family of injection paths.
  • KVM software (Synergy, MWB inbound) used by accessibility users.

If any of those rely on the centralized hook to surface PowerToys hotkeys (e.g. user triggers a PT shortcut via OSK), this becomes a silent accessibility regression that only surfaces in the field.

A manual smoke note in the PR body "verified Win+ via OSK still triggers" would close it out cleanly.

- Tag dummy 0xFF SendInput with dwExtraInfo to prevent cross-hook interference
- Skip LLKHF_INJECTED events from OSK/macro tools (dwExtraInfo==0)
- Revalidate key state in PressedKeyTimerProc before firing action
- Kill pending timers and reset state in Stop()
- Make vkCodePressed atomic to fix data race

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@crutkas crutkas force-pushed the fix-stuck-keys/b-centralized-hook branch from 1fd96dc to 61b7234 Compare June 12, 2026 22:51
@crutkas

crutkas commented Jun 12, 2026

Copy link
Copy Markdown
Owner Author

Removed the injected-event filter entirely rather than narrowing it.

The hook already passes PowerToys' own injected events through at the top of KeyboardHookProc via the CENTRALIZED_KEYBOARD_HOOK_DONT_TRIGGER_FLAG check, so the new (LLKHF_INJECTED && dwExtraInfo == 0) filter only ever caught third-party injection — On-Screen Keyboard, Narrator, AT/eye-gaze tools, KVM switches, macro tools — and stopped PowerToys shortcuts from firing for them. That's a pure accessibility regression with no stuck-key benefit, and it can't be narrowed reliably (OSK injection is indistinguishable from macro injection by flags).

The actual hardening stays: atomic vkCodePressed, re-validating the key is still physically held (GetAsyncKeyState) before firing a pressed-key timer, and killing pending timers in Stop() before unhooking.

@crutkas

crutkas commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

Superseded by the upstream PR against microsoft/PowerToys: microsoft#48570 (rebased onto current main, builds/tests verified). Closing this intra-fork review PR.

@crutkas crutkas closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants