#451 fix(auth): refuse GUI auth dialog off the main thread on macOS (#399)

[Anonymous]

Originally created by: harshitagrawal2O

Pull Request

Slice

• Repo: doberman-core
• Feature / Slice: [#399] — [bug] AUTH-tier decisions execute without human confirmation in the Claude Code host-hook path (fail-closed violation)
• Plan reference: n/a (bug fix)

What this PR does

Traced the reported bug to its root cause and fixes it.

Root cause: Every real caller of the auth-challenge prompter chain runs it on a background daemon thread — run_auth_challenge dispatches through _run_with_deadline (src/doberman/auth/challenge.py), which spawns a threading.Thread specifically so a wall-clock deadline can be enforced on a channel that might otherwise block forever; 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 (macOS) 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 on X11 — it can silently fail to render (no window ever appears, but the call also never raises) or abort the whole process. Either outcome could leave an AUTH-tier decision recorded as approved with no human ever having seen a dialog — exactly the fail-closed violation reported, and consistent with every piece of evidence in the issue:

• BLOCK verdicts never touch this code path at all (decision_payload returns synchronously, no Tk) — matching the report's control group, which worked every time.
• AUTH always resolved auth=executed with zero denials/timeouts across weeks of history — consistent with a near-instant native-level failure rather than a slow hang that the 120s dialog timeout or 1200s outer deadline would otherwise catch and log as timeout.
• macOS-specific, matching the report's platform.

Fix: _open_root() (src/doberman/auth/gui_prompter.py) now refuses — raising the existing PrompterUnavailableError — before ever importing tkinter when it detects sys.platform == "darwin" and the current thread is not the main thread. This slots into the already-correct existing contract: FallbackPrompter falls through to the terminal prompter, and if that is also unavailable, resolve_auth/the provider denies with the existing, already-worded "approval dialog could not be shown" message. Windows and Linux are completely unaffected — the guard only fires on the one platform where this is a documented hazard, and only when genuinely off the main thread.

I could not reproduce the live macOS symptom directly (no macOS hardware), but the fix removes the hazardous call path unconditionally regardless of exactly which native failure mode manifests, and is covered by tests proving the guard fires (including via a real background threading.Thread, not just mocked thread identity) and that the rest of the fallback/deny chain behaves correctly once it does.

Tests added (run in CI)

• tests/unit/test_gui_prompter.py — 9 new tests:
• the guard refuses before tkinter.Tk() is ever called (mocked thread identity, and via a real background threading.Thread)
• inert on the real main thread (macOS) — tkinter.Tk() is still attempted, proving the guard checks thread affinity, not platform alone
• inert on non-macOS platforms (win32, linux) even off the main thread — zero behavior change for those users
• reachable through the public GuiPrompter.confirm() API, not just _open_root()
• full-chain: FallbackPrompter([GuiPrompter(), tty]) falls through to the terminal when GUI is refused
• full-chain: LocalAuthProvider.authenticate() denies (never approves) when both GUI and terminal are unavailable under the exact [#399] conditions

Public-release safety (doberman-core only)

• [x] Contains nothing from the "not allowed" list: no enterprise/hosted code, no proprietary detection, no customer data, no secrets, no commercial-license code
• [x] Core still builds/tests/runs with NO enterprise package installed

Security checklist

• [x] Fails closed on error / uncertainty — the exact point of this fix: an unsafe channel now reports itself unavailable instead of risking undefined behavior
• [x] No secret, full file, or unredacted prompt logged or committed
• [x] Any guardrail/learning change is raise-only (no silent loosening) — this only removes a way an AUTH could resolve as approved without a real answer; it never turns an existing approval path into a denial under normal (main-thread-safe) conditions
• [x] Every BLOCK/AUTH carries reason codes + a human explanation — unchanged; this fix is upstream of verdict formation, in the challenge-resolution layer
• [x] doberman-core does not import doberman_enterprise

Edge cases covered / Deviations from plan / Risks introduced

• Could not reproduce on macOS hardware — this fix is based on static tracing of the code path plus the well-documented Cocoa/Tk main-thread requirement, not a live repro. Flagging this explicitly per the issue's own request for maintainer instrumentation; happy to iterate further if a live repro surfaces a different or additional failure point.
• Secondary observation, not fixed here (out of scope for this PR): TtyPrompter._open_tty() (src/doberman/auth/tty_prompter.py) raises a bare OSError rather than PrompterUnavailableError when no controlling terminal is attached. This does not cause a fail-open — LocalAuthProvider.authenticate()'s own broad except Exception still catches it and denies — but it means TtyPrompter doesn't participate in FallbackPrompter's "try the next channel" semantics the same way GuiPrompter does. Low severity today (it's the last channel in the chain), but worth a follow-up if a channel is ever added after it.
• No behavior change for Windows/Linux, or for macOS on the main thread — verified by dedicated tests above.

[Anonymous]

Ticket changed by: fu351

• status: open --> closed

[Anonymous]

Originally posted by: fu351

Merged, thanks @harshitagrawal2O! Raising the guard before import tkinter is the part I want to call out, it turns a Cocoa-level crash we can't reliably catch into a plain Python exception at the seam, and the test that spins up a real background thread instead of mocking thread identity proves the detection actually works. One note on the mechanics: a dependency pin I merged put a changelog conflict on your branch, so I landed this through [#453] rather than pushing anything to your fork, your commits are in exactly as you wrote them. I'm keeping [#399] open: this closes the likeliest path, but the original symptom still needs confirming on real macOS hardware before I call the bug dead. [#198] is assigned to you whenever you're ready.