land #451: refuse GUI auth dialog off the main thread on macOS - #453
Merged
Conversation
Every real caller of the auth-challenge prompter chain runs it on a background daemon thread: run_auth_challenge dispatches through _run_with_deadline (a spawned threading.Thread, so a wall-clock deadline can be enforced on a channel that might never return), and the MCP-proxy path does the equivalent via asyncio.to_thread. GuiPrompter therefore always constructs its Tk() root off the process's main thread. Cocoa's Tk backend requires its NSApplication event loop to start on the real OS main thread. Constructing Tk() off it is a documented hazard that does not reliably surface as a catchable TclError the way a missing $DISPLAY does -- it can silently fail to render, or abort the process -- either of which could leave an AUTH-tier decision recorded as approved with no human ever having seen a dialog: a fail-closed violation. _open_root() now refuses (raises PrompterUnavailableError) before ever importing tkinter when it detects sys.platform == "darwin" and the current thread is not the main thread. This is exactly the existing "channel unavailable" contract: FallbackPrompter falls through to the terminal, and if that is also unavailable, the provider denies. Windows and Linux are unaffected -- the guard is scoped to the one platform where this is a documented hazard, and only fires when actually off-main-thread. 9 new tests: the guard fires before tkinter is touched (mocked and via a real background thread), is inert on the real main thread, is inert on non-macOS platforms, is reachable through the public GuiPrompter API, and the full FallbackPrompter/LocalAuthProvider chain still denies (never approves) when both GUI and terminal are unavailable on macOS.
# Conflicts: # CHANGELOG.md
7 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
2 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.
Lands #451 with the author's commits unchanged (@harshitagrawal2O keeps authorship; the PR flips to Merged when this lands). The only addition is a merge of current main resolving the CHANGELOG anchor conflict against #452's river bullet. No code differences from #451.