fix: handle AttachThreadInput Access Denied for elevated processes#123
Conversation
AttachThreadInput raises Access Denied if targeting elevated processes (e.g. Settings, Task Manager) due to Windows UIPI. Catch the exception per-thread and continue with SetForegroundWindow so the window can still be brought to the foreground.
There was a problem hiding this comment.
Pull request overview
This PR updates the Windows desktop focus-switching flow to tolerate win32process.AttachThreadInput failures (notably “Access Denied” when the target window belongs to an elevated/high-integrity process), allowing the function to continue attempting SetForegroundWindow/BringWindowToTop/SetWindowPos.
Changes:
- Wrap
AttachThreadInputin a per-threadtry/exceptto avoid aborting the entire focus-switch flow on failure. - Add debug logging when the thread attach fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| win32process.AttachThreadInput(current_tid, thread, True) | ||
| attached_threads.append(thread) | ||
| except Exception as e: | ||
| # AttachThreadInput fails with Access Denied for elevated | ||
| # processes (e.g. Settings, Task Manager). Skip the attach | ||
| # and still attempt SetForegroundWindow below. | ||
| logger.debug( | ||
| f"AttachThreadInput failed for thread {thread} " | ||
| f"(likely elevated process), skipping: {e}" | ||
| ) |
There was a problem hiding this comment.
The new handler catches all Exceptions but logs it as "likely elevated process". That message can be misleading for unexpected failures (e.g., invalid TID) and it also risks masking real bugs. Consider catching the specific win32/pywintypes error and only continuing when the failure is Access Denied (winerror==5); otherwise re-raise or log at a higher severity.
| # AttachThreadInput fails with Access Denied for elevated | ||
| # processes (e.g. Settings, Task Manager). Skip the attach | ||
| # and still attempt SetForegroundWindow below. | ||
| logger.debug( |
There was a problem hiding this comment.
This is logged at DEBUG, but this module sets logger.setLevel(logging.INFO) at import time, which will suppress this message in most configurations. If you want this failure to be observable when it happens, either avoid hard-setting the module logger level, or log this at INFO/WARNING (keeping elevated-window noise in mind).
| logger.debug( | |
| logger.info( |
| try: | ||
| win32process.AttachThreadInput(current_tid, thread, True) | ||
| attached_threads.append(thread) | ||
| except Exception as e: | ||
| # AttachThreadInput fails with Access Denied for elevated | ||
| # processes (e.g. Settings, Task Manager). Skip the attach | ||
| # and still attempt SetForegroundWindow below. | ||
| logger.debug( | ||
| f"AttachThreadInput failed for thread {thread} " | ||
| f"(likely elevated process), skipping: {e}" | ||
| ) | ||
|
|
||
| win32gui.SetForegroundWindow(target_handle) | ||
| win32gui.BringWindowToTop(target_handle) |
There was a problem hiding this comment.
There are unit tests for windows_mcp.desktop.service.Desktop that rely on mocking (e.g., in tests/test_snapshot_display_filter.py), but this new error-handling path isn’t covered. Add a test that mocks win32process.AttachThreadInput to raise Access Denied and asserts the function still calls SetForegroundWindow/BringWindowToTop and doesn’t propagate the exception.
|
In that case, if we run the MCP server in admin mode, we shouldn't encounter any issues. I had a similar experience when I tried to access the elements in the Task Manager; it failed, but it worked in admin mode. |
Summary
bring_window_to_topnow gracefully handlesAttachThreadInputfailures when targeting elevated (high-integrity) processes such as Settings and Task ManagerSetForegroundWindow/BringWindowToTop/SetWindowPosProblem
When
bring_window_to_topis called with a window handle belonging to an elevated process (e.g. Windows Settings, Task Manager),win32process.AttachThreadInputraises an Access Denied exception due to Windows UIPI (User Interface Privilege Isolation). This causes the entire window-switching operation to fail, even though the subsequentSetForegroundWindowcall can still bring the window to the foreground without the thread attach.Solution
Wrap the
AttachThreadInputcall in atry/exceptblock so that:DEBUGlevel with the exception details, and the flow continues to callSetForegroundWindow+BringWindowToTop+SetWindowPos.Changed files
src/windows_mcp/desktop/service.py—bring_window_to_top: added per-threadtry/exceptaroundAttachThreadInput