Feat/uac policy#309
Open
bensheed wants to merge 159 commits into
Open
Conversation
The issue described the secure-desktop host service as one possible privileged feature; nest its commands under `service secure-desktop` instead of flat `service install` so the name describes what the service is for and leaves the `service` namespace open for future helpers: uv run windows-mcp service secure-desktop install uv run windows-mcp service secure-desktop uninstall uv run windows-mcp service secure-desktop start / stop / status Updates the followup hint in `install` and the section header to match. No behaviour changes — only the CLI surface.
…e-desktop install' Two source-level docstrings still referenced the old flat path.
`_build_uac_tree_state` references `Center` but the class was never imported, so the UAC tree path crashes with NameError as soon as the service-routed UAC handling is triggered. This was introduced when `_build_uac_tree_state` was added but the import was missed.
The previous attempt landed on disabling the "Switch to secure desktop" policy (PromptOnSecureDesktop=0) so UAC dialogs would render on the Default desktop where user-mode UIA could reach them. The issue body calls this out explicitly: "Disable UAC entirely — bad security posture, breaks anything that detects UAC level (Edge, Defender, MDM tools)." It is also a confession that the cross-desktop service path didn't work in practice. Removing the workaround forces us to make the proper path (Service running as LocalSystem, SetThreadDesktop to follow the input desktop, UIA against Winlogon) actually work. This commit only removes the registry mutation. Subsequent commits add the policy env var, the WaitForUACPrompt MCP tool, Type/Drag routing through the service, the pipe SID DACL, and the %ProgramFiles% install location that make the proper path safe to ship.
…forcement The Secure Desktop host service now refuses to auto-click UAC prompts unless an explicit policy permits it. Policy values (per issue CursorTouch#236): block (default) service shows the UAC dialog to the agent but refuses to invoke Yes/No. Human approves. allow_with_match auto-invoke only when the UAC dialog's "Verified publisher" substring-matches one of the persisted publisher allowlist entries. allow_all auto-invoke any UAC prompt. Opt-in. Only sensible inside sandboxed VMs. Implementation notes: - service/policy.py: SecureDesktopPolicy dataclass + registry persistence at HKLM\SOFTWARE\Windows-MCP\SecureDesktop. The service reads at request time so the broker cannot bypass enforcement. - service/secure_desktop.py: new uia_type_at, uia_drag_from_to, get_uac_publisher (best-effort English publisher extraction from the UAC dialog's UIA tree), wait_for_uac_prompt (blocks until the input desktop becomes Winlogon). - service/host.py: _enforce_policy() runs before any auto-input op when the input desktop is Winlogon. Read-only ops (screenshot, tree) are never gated — agents always need visibility. - service/pipe.py: client methods for the new pipe verbs. - __main__.py: `service secure-desktop install` accepts --policy and --allow-publisher; precedence is CLI > env > config.toml > "block". New `service secure-desktop set-policy` command changes policy without reinstalling. Uninstall now also clears the registry key. - infrastructure/config.py: SecureDesktopConfig section in TOML.
The agent-facing half of the secure-desktop story. The tool blocks for
up to timeout_ms (default 60 s) until the input desktop becomes
Winlogon — i.e. a UAC consent dialog fires — then returns:
desktop "Winlogon"
publisher "Verified publisher" string from the UAC dialog, or None
when the layout doesn't match the English regex.
tree full UIA tree of the consent dialog so the agent can read
the program name, location, and Yes/No buttons.
policy current Secure-Desktop consent policy + allowlist, so the
agent can pre-decide whether an auto-click will succeed.
When the host service is not installed, returns a structured error
explaining how to install it rather than silently blocking — the
broker cannot reach the Secure Desktop on its own.
Mirrors what Click already does. When the input desktop is Winlogon (UAC active), the broker delegates to the LocalSystem host service because keyboard/mouse injection from the user session is dropped by UIPI on the Secure Desktop. - Type uses IUIAutomationValuePattern.SetValue (single atomic write, so caret_position/clear/press_enter are ignored on this path — agents should re-screenshot to verify). - Drag uses IUIAutomationTransformPattern.Move (best-effort, only works when the source supports the transform pattern). UAC dialogs rarely need drag, so this is here for completeness. Both calls go through the policy-gated dispatcher in the host service, so block / allow_with_match / allow_all are honoured.
The previous attempt left a NULL DACL on the named pipe, admitting in a code comment that "the tighter SYSTEM+user DACL can be added later once the basic pipe works." That left the service open to any local process, contradicting the issue's requirement that the pipe "require a SID match against the interactive console user." This commit builds a proper SECURITY_DESCRIPTOR per pipe instance: - SYSTEM SID (the service itself) always granted FILE_ALL_ACCESS. - Active console user SID, resolved via WTSGetActiveConsoleSessionId → WTSQueryUserToken → GetTokenInformation(TokenUser), granted FILE_ALL_ACCESS. - If no user is logged on yet (boot before login), falls back to SYSTEM + BUILTIN\Administrators so a local admin can still test. - On any failure to build the ACL, falls through to an *empty* DACL (deny-everyone-except-owner) rather than a NULL DACL — failures are loud, not silent. Caveat: the DACL on a given pipe instance is fixed at creation. If the console user changes mid-service-lifetime (e.g. logout/login of a different account), the broker may briefly hit access-denied until the service rotates to a new pipe instance. Recreating per client (which the loop already does) makes this a single-attempt issue in practice.
Per the issue requirement that "the install location must be ACL'd correctly … anyone who can write to the service binary path now has SYSTEM," the install command now checks that both: - sys.executable - windows_mcp.__file__ live under a default admin-only prefix (%ProgramFiles%, %ProgramFiles(x86)%, %SystemRoot%). If either is in a user-writable path (typical for uv tool installs, per-user pip installs, or venvs in %LOCALAPPDATA%), the install is refused with instructions to install Python+windows-mcp system-wide. This is a heuristic, not a true ACL check — but it covers the common case and produces an actionable error. For disposable VMs / development, --allow-user-binary-path opts out of the check with a warning. The VM test will use this flag. Rejected alternative: copying the venv into %ProgramFiles%. That works for plain venvs but breaks for uv tool installs (which use redirector caches), and the copy adds substantial install-time complexity. The check-and-refuse approach delivers the security guarantee with a fraction of the surface area.
End-to-end test of the LocalSystem secure-desktop story. Driven via
vncdotool from Linux to kick off run_all.ps1 inside the Windows VM,
then results.json comes back over the SMB share.
Pieces:
- bringup.sh Linux orchestrator. Waits for the desktop to be
visible on VNC :5900, opens powershell via Win+R,
pastes the command line that runs run_all.ps1 from
the share, then waits for results.json to appear
at the corresponding Linux path on the bind-mount.
- run_all.ps1 Windows-side orchestrator. Bootstraps Python/uv,
stages the repo locally (uv won't sync to a UNC
path), installs the service with
--allow-user-binary-path (it's a disposable VM),
verifies the service is RUNNING, then hands off
to mcp_client.py.
- mcp_client.py Real MCP client using the `mcp` SDK. Spawns
`windows-mcp serve --transport stdio` as a child
and talks to it. Lists tools, calls
WaitForUACPrompt while a side-process triggers a
UAC dialog via `Start-Process -Verb RunAs`, asserts
the tool returns a non-empty UIA tree. Writes a
JSON report.
This is path A (in-VM driver, stdio transport). Path B (Linux-side
HTTP client) comes next — same harness but with --http flag.
Same mcp_client.py, called with --http, against the MCP server running over streamable-http inside the VM. Validates the full network path: Linux → docker host port → container → socat forward → VM → MCP server. Documents the one-time setup (container restart with -p 8000:8000, socat forward inside the container) and reads WINDOWS_MCP_URL to let the user point it elsewhere if needed.
vncdotool's command is 'type', not 'typewrite' (which silently dropped all keystrokes). Also switched from Win+R to the Start-menu search, which is more deterministic — Win+R + Enter on an empty box was randomly launching netsh because of Windows' recent-commands fallback.
Wraps the long PowerShell -ExecutionPolicy Bypass -File <UNC> command in a single short batch file so the Win+R driver only has to type one short UNC path.
…rcement The previous version only asserted that WaitForUACPrompt returned a tree. That doesn't prove the feature works — the agent must be able to act on the dialog. New per-assertion checks: list_tools includes WaitForUACPrompt WaitForUACPrompt returns dialog after UAC fires policy is <expected> UAC tree contains invokable Yes button (allow_all) Click(Yes) dismisses UAC (block) Click(Yes) is refused with policy denied run_all.ps1 now runs the suite twice — once with policy=allow_all (must click Yes successfully) and once with policy=block (must be refused). Phase results merged into results.json.
…path) Same content as kickoff.bat. The reason for the duplicate name is that vncdotool's shift-minus rendered as plain dash on the Win11 VM keyboard layout — typing 'vm_e2e' came out as 'vm-e2e' and the path failed. This file is for the path-without-underscores invocation if we end up copying it to a no-underscore location.
… PATH Two failure modes seen in the first VM run: 1. `Get-Command python` returned the Microsoft Store stub (Win11 ships an App Execution Alias of that name that just opens the Store). `python --version` then printed nothing and the script continued thinking python was installed. Replaced with Test-RealPython which only counts as installed if --version actually returns a Python X.Y string. 2. `uv` install ran in a child PowerShell via -c, so any $env:Path mutations the installer made never reached the parent session. Switched to in-process Invoke-Expression of the install script, then probe a known list of install locations and prepend the right one to PATH. Both functions now throw on failure instead of silently continuing, so results.json reports the real cause.
PowerShell 5.1 (default in Win11) negotiates TLS 1.0/1.1 unless told otherwise. Astral's CDN (and most modern HTTPS endpoints) reject that and irm fails with 'Could not establish trust relationship'. Set SecurityProtocol explicitly to include Tls12 before the irm call.
Saw transient 'Could not find file' from Set-Content/Add-Content against a UNC log path on Win11 — likely SMB-side caching or a quirk of the LanmanRedirector after the file was deleted host-side. Writing locally to %TEMP% and copying to the share on every line is robust and only marginally noisier.
winget on a fresh dockur Win11 fails with 'Data required by the source is missing' (broken first-run source bootstrap). The 'python' on PATH is the Microsoft Store stub that does nothing. Skip both. Install uv first (self-contained binary from Astral CDN, TLS 1.2 forced), then 'uv python install 3.13' downloads and pins a real CPython under uv's own cache. uv sync after that uses the managed interpreter. Zero Windows-side Python machinery.
The Linux sandbox running dockur intercepts outbound TLS with a custom CA the Windows VM doesn't trust. Real Win11 boxes won't see this — but inside our VM, the same fix curl needed (mount host CA bundle) isn't available, so we punt and accept all certs FOR THIS RUN_ALL.PS1 only. The script runs in a disposable test VM, and the loaded payload is the uv install script from a known-good Astral URL we already trust. Production users running this on real Windows don't hit this code path because production callers don't run run_all.ps1 — it's purely the test harness.
…tstrap) The disposable test VM can't trust the sandbox's MITM TLS cert, so any HTTPS download from inside Windows fails. Pre-stage uv.exe and the python.org Python 3.13 installer in the share (downloaded host-side where TLS trust is set up) and have run_all.ps1 copy them into the VM instead of downloading. bin/ is gitignored because the staged binaries are ~90 MB; they're regenerated host-side via: curl -sL -o /tmp/u.zip https://github.com/astral-sh/uv/releases/latest/download/uv-x86_64-pc-windows-msvc.zip unzip -o /tmp/u.zip -d tests/manual/vm_e2e/bin/ curl -sL -o tests/manual/vm_e2e/bin/python-install.exe https://www.python.org/ftp/python/3.13.0/python-3.13.0-amd64.exe
$ErrorActionPreference=Stop causes PowerShell to throw on the first stderr line from a native exe, even when the exit code is 0 (uv prints its python-detection info to stderr). Added Invoke-Native helper that locally switches ErrorActionPreference, captures stdout+stderr to a log file, mirrors to the share, and only throws on non-zero exit code.
uv (rustls) hit 'invalid peer certificate: UnknownIssuer' against files.pythonhosted.org because the sandbox proxy intercepts TLS with a CA the VM doesn't trust. Same disposable-VM situation as the .NET cert bypass earlier — set UV_INSECURE_HOST for the needed hosts only.
Windows' default cp1252 codec can't encode '\u2192' (right arrow) or '\u2014' (em-dash) when stdout/stderr is piped to a non-tty. The VM test harness pipes the output to Tee-Object → uv sync → cp1252 encode → UnicodeEncodeError → exit 1. Replaced with -> and -- across the CLI for piping safety. Visible output is unchanged on TTYs; pipes no longer crash.
Previous run leaves WindowsMCPHost service running. Its python.exe
is locked, so Remove-Item -SilentlyContinue skips the .venv
contents, leaving a stale venv that confuses uv on the next run
('failed to locate pyvenv.cfg'). Stop and delete the service first.
Dockur's autounattend sets EnableLUA=false, which kills UAC entirely — Start-Process -Verb RunAs auto-elevates with no prompt and nothing fires on the Secure Desktop. The whole secure-desktop story can't be tested in that state. run_all.ps1 now detects EnableLUA != 1 and: 1. Sets EnableLUA=1 + ConsentPromptBehaviorAdmin=5 (default) 2. Registers an ONLOGON scheduled task to re-run itself 3. shutdown /r — auto-login resumes the test with UAC live 4. On the resumed run, deletes the task so it doesn't re-fire
…tion) `schtasks /Delete /TN <name>` writes 'cannot find file' to stderr when the task doesn't exist. PowerShell with ErrorActionPreference=Stop treats that as a fatal exception. Wrapping in cmd.exe /c with stderr redirected to nul gives us idempotent delete-if-present without the exception.
…tion detail Previous detail only showed fired=False, hiding whether it was a timeout, a service-not-available, or a host-call error. Surface ok/fired/reason/error fields explicitly.
Observed in the VM run: ping (is_available) succeeded, then the immediately-following wait_for_uac_prompt _call() failed with WaitNamedPipe ERROR_FILE_NOT_FOUND. The host service spends ~20-50 ms between accepting one connection and creating the next pipe instance; back-to-back broker calls land inside that window and Windows returns 'file not found' (not 'busy/timeout') when no instance is in WAITING_FOR_CONNECT state. Wrap WaitNamedPipe + CreateFile in _open_with_retry that retries on errors 2 (FILE_NOT_FOUND) and 231 (PIPE_BUSY) for up to 30 attempts × 100 ms = 3 s before giving up.
This reverts commit 8a164ed.
…o children" This reverts commit 23310c2.
…rom walker Previous result: top_windows=3, first node='User Account Control' inv=True with NO children. consent.exe renders the Yes/No buttons via XAML/ Composition that the UIA tree-walker can't descend into across the integrity boundary (consent.exe runs at System integrity; the user-session worker has UIAccess but UIA's walker still filters those elements out). Synthesize Yes/No nodes with predicted center coords inside the dialog bbox so the LLM's button-finding pass (name + can_invoke) succeeds. The follow-up Click(loc=[cx, cy]) uses ElementFromPoint at click time -- that path resolves XAML elements cross-integrity (UIAccess can read at any pixel) even when the walker can't enumerate them. Pure-data approach (no COM calls during tree assembly) -- reverted ElementFromPoint probe in 7a1915a was suspected of hanging the worker.
Order the synth block before the _diag string assembly so the diag captures synthesized_yes_no=N + bbox of the dialog when synth fires. Previously the diag string was frozen on the first node before synth ran, so client-side dumps showed top_windows=N but no indication of whether the Yes/No nodes came from the walker or the synth fallback.
The broker runs at medium integrity; consent.exe runs at System integrity. UIPI blocks input from lower-to-higher integrity, so uia.Click at the synthesized Yes/No coords silently fails to dismiss UAC. The host service already exposes a UIAccess-tokened user-session worker via the pipe RPC method "uia_click_at" -- it calls IUIAutomation. ElementFromPoint + InvokePattern, which DOES cross the integrity boundary because the worker token has TokenUIAccess=1. In Desktop.click, detect via WindowFromPoint + GetWindowThreadProcessId whether the click target is owned by consent.exe. If so, fan out to client.uia_click_at and skip the local uia.Click. Falls back to local click if the host service isn't available or the worker call fails. Only affects the consent.exe code path -- normal app clicks bypass the host-routing branch entirely.
…tons consent.exe renders its Yes/No buttons as XAML/Composition that expose no UIA InvokePattern, so ElementFromPoint(x,y).GetCurrentPattern(Invoke) returns None and the programmatic invoke can't activate them. Add _send_mouse_click(x,y): an absolute-coordinate SendInput left-click normalised across the virtual desktop. The user-session worker holds TokenUIAccess=1, which exempts its synthetic input from UIPI, so the click reaches the System-integrity UAC dialog. uia_click_at now tries InvokePattern first (works for normal windows), then falls back to the physical click when the pattern is absent -- which is the consent.exe case the synthesized Yes/No nodes target.
…strators WindowsMCPHost is SERVICE_AUTO_START, so it comes up at boot before anyone logs in. The pipe server rebuilds the security descriptor per instance and then blocks on ConnectNamedPipe, so the first boot-time instance keeps its DACL until a client connects. The old fallback (SYSTEM + BUILTIN\Administrators) deadlocked that instance: the broker runs in the interactive session under the user's *filtered* medium-integrity token, where Administrators is a deny-only SID, so it could never open the pipe. ConnectNamedPipe never completed, the loop never recreated the instance with the correct console-user DACL, and the service sat unreachable until a manual restart (the old restart-host workaround). Fall back to SYSTEM + NT AUTHORITY\INTERACTIVE (S-1-5-4) instead. INTERACTIVE is present in every interactively-logged-on token, filtered or not, so the boot-time pipe becomes reachable the moment the user logs in; once a console user is resolved, later instances tighten to SYSTEM + that specific SID. The operations behind the pipe remain policy-gated regardless.
The sandbox MITM proxy intermittently times out mid-download ("error
decoding response body / operation timed out"), failing an otherwise-fine
uv sync on a single package (e.g. pyperclip). uv caches everything it did
fetch, so a retry only re-downloads the flaked package. Wrap the sync in a
5-attempt loop with linear backoff before failing setup.
The MCP server's first cold-start under a KVM-less TCG VM (~10x slower) pulls in a heavy import chain and can exceed the old 120s deadline while still starting, producing a spurious "server never came up" failure. Bump the wait to 300s. On genuine failure, dump the server's own error/std logs + schtasks state to server-diag.txt on the share so the run self-diagnoses instead of requiring live VNC archaeology. Also ignore the run-test-now / server-diag dev launchers.
Root-caused the "WaitForUACPrompt fired=False" failure on the TCG VM: the
trigger spawned powershell -> Start-Process -Verb RunAs, and powershell's
cold start alone took long enough (tens of seconds, KVM-less) that the
consent dialog didn't appear until after the client's 30s WaitForUACPrompt
wait had already timed out. UAC itself works and renders on the Default
desktop (POSD=0) exactly as intended.
Two changes:
- Fire the prompt directly via ShellExecuteW("runas", "cmd.exe") on a daemon
thread instead of spawning powershell. The dialog now appears in ~a second.
A small _Trigger wrapper preserves the old .wait()/.kill() interface.
- Bump the primary WaitForUACPrompt server-side poll to 180s and pass a 200s
client read-timeout (via a new read_timeout arg on call()), so a slow
broker round-trip can't make the transport give up first. The post-click
"UAC dismissed?" check gets a 60s read-timeout too.
…st log Root-caused the "WaitForUACPrompt call timed out after 200s" failure on the TCG VM (UAC fired and rendered on the Default desktop correctly, but the call never returned): the consent-walk retry loop ran a fixed 30 attempts with a 15s per-spawn timeout, unbounded by the caller's deadline. Each worker spawn is a fresh Python + comtypes COM init, which is slow on a KVM-less VM, so the loop could run for minutes -- well past the client's read timeout -- and the dialog was never returned to be clicked. Changes: - secure_desktop: make the consent-walk loop deadline-aware. Each spawn's timeout is capped to the remaining budget, and the loop stops once the overall deadline is near, so WaitForUACPrompt always returns within timeout_ms instead of hanging. Publisher lookup is likewise skipped if no time is left. - host.py: log to %ProgramData%\windows-mcp (Users-readable) instead of SYSTEM's %TEMP%, so the interactive user / test harness can read the service log without an elevated dump. Falls back to %TEMP%. - setup.ps1: pre-warm the comtypes UIAutomationCore gen cache into the shared venv after uv sync, so the first worker spawn doesn't pay the (slow, one-time) wrapper-generation cost during the first WaitForUACPrompt. - test.ps1: copy the host service log to the share on every run (now that it's readable), for self-diagnosis.
…ementFromHandle Live run reached 3/4: WaitForUACPrompt returned the "User Account Control" window in ~21s (fired=True), but the Yes-button assertion failed because the worker resolved the dialog via the GetFocusedElement+walk-up path, which returned before the ElementFromHandle branch where the Yes/No synthesis lived -- so the window came back childless. Hoist the synthesis into a shared _ensure_uac_buttons() helper and apply it on every path that returns the consent window (FocusChanged event, GetFocusedElement walk-up, FindAll, ElementFromHandle). Whichever strategy finds the dialog, the childless window now gets synthesized Yes/No buttons with predicted center coords.
Host-log forensics of the repeated top_windows=0 failures: WaitForUACPrompt correctly retried the worker 9x and returned within its deadline, but every worker returned an EMPTY tree, so the dialog was never surfaced. Root cause: uia_get_tree runs its walk via _run_on_fresh_thread with the default 15s timeout. On a KVM-less TCG VM the consent walk (a ~4s FocusChanged wait + several cross-integrity COM calls) plus the fall-through full-desktop root-walk exceeds 15s, so the thread times out and returns None -> [] -> top_windows=0. The one run that passed only did so because GetFocusedElement happened to resolve before 15s. Three fixes: - When scoped to a consent_pid and all strategies miss, return a diagnostic placeholder immediately instead of falling through to the expensive full-desktop root walk (which is useless here -- consent.exe isn't in it -- and is the main time sink that blows the thread budget). - Give uia_get_tree's worker thread 40s (kept under the broker's per-spawn timeout) when consent-scoped, so the strategy sequence can finish. - Trim the mostly-futile FocusChanged wait from 4s to 2s so the reliable ElementFromHandle path runs sooner within the budget.
4/5 on the live VM: WaitForUACPrompt returns the dialog, the synthesized Yes button is found, Click routes correctly to the UIAccess worker (op=uia_click_at -> op=click_at 535 520, policy=allow_all). But the worker threw "expected POINT instance instead of _POINT": comtypes' ElementFromPoint rejects our local ctypes _POINT struct, and because that call wasn't guarded it aborted uia_click_at *before* the SendInput fallback that actually dismisses the consent dialog. Pass ElementFromPoint a plain tuple (comtypes marshals it) and wrap it in try/except so any failure falls through to _send_mouse_click. For the consent.exe XAML buttons that route here there's no InvokePattern anyway, so the physical UIPI-exempt click is the real path.
The UAC see-and-click feature (issue CursorTouch#236) only needs three host operations: wait_for_uac_prompt (see the dialog), uia_click_at (click Yes/No), and the desktop_name/policy_state status reads. The branch had grown a full speculative UIA write API around that — none of it reachable from any MCP tool or the e2e suite. Removed end to end (worker op → host dispatch case → pipe client method → secure_desktop function): - uia_invoke / uia_invoke_element - uia_type_at (+ _UIA_ValuePatternId, _POINT struct) - uia_drag_from_to - uia_windows / uia_get_window_titles (+ _UIA_NamePropertyId) Also removed the dead uia_tree / get_uac_publisher pipe methods and host dispatch cases — wait_for_uac_prompt already calls those worker ops directly via _spawn_in_user_session, so nothing external invoked them. The underlying worker ops and functions stay. Other dead code: - _input_desktop's ignored prefer_winlogon stub arg - iter8-probe.ps1 / iter8-test.ps1 one-off investigation scaffolding (unreferenced by run_all/setup/test) - stale protocol.py method docstring (listed a screenshot method that was never implemented, plus the now-removed ops) Net -565 LOC. No behavior change to the tested path; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ubSxdAaDkQxYJ17U2zuDU
test.ps1 already copies mcp_client.py from the share each run so harness edits land without re-running setup.ps1's full robocopy. Extend the same idea to the windows_mcp package: robocopy share/src into the installed venv before invoking the client, so the reboot-test validates the current source rather than whatever setup.ps1 last installed. The user-session worker is spawned fresh per WaitForUACPrompt, so it picks up the synced code on the next call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ubSxdAaDkQxYJ17U2zuDU
_enforce_policy short-circuited to "allowed" unless the input desktop reported "Winlogon". But with PromptOnSecureDesktop=0 the consent dialog renders on the Default desktop, and the input-desktop name flips between "Default" and "Winlogon" while the prompt is up — so block / allow_with_match enforced only intermittently (whichever value the racy read happened to return at click time). Every click routed to the host is a consent-dialog click — the broker only forwards clicks whose target pixel is owned by consent.exe — so the consent policy is always the deciding factor. Drop the desktop check and enforce unconditionally, so block reliably refuses and allow_with_match reliably gates on the verified publisher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ubSxdAaDkQxYJ17U2zuDU
The manual VM e2e harness (tests/manual/vm_e2e/) and the two feature/ investigation docs were development tooling, not part of the shipped feature. Removing them keeps the merge to the necessary code only. The harness stays available in git history if the e2e run ever needs to be reproduced. Also reverts the .gitignore entries that only covered those dev/test artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ubSxdAaDkQxYJ17U2zuDU
Secure Desktop / UAC support — let an agent see and click UAC consent dialogs (CursorTouch#236)
PR Summary by QodoAdd Secure Desktop service mode for UAC policy + WaitForUACPrompt
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
15 rules 1. Pipe allows INTERACTIVE clients
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+141
to
+199
| def _build_pipe_sa() -> Any: | ||
| """Return a SECURITY_ATTRIBUTES that allows only SYSTEM + the console user. | ||
|
|
||
| Falls back to SYSTEM + NT AUTHORITY\\INTERACTIVE (S-1-5-4) when no console | ||
| user is logged in yet — the typical case at boot, because the service is | ||
| SERVICE_AUTO_START and comes up before anyone logs in. The pipe server | ||
| loop rebuilds this SD for every instance and then blocks on | ||
| ConnectNamedPipe, so the *first* boot-time instance keeps whatever DACL it | ||
| was created with until a client actually connects. If that fallback were | ||
| BUILTIN\\Administrators (the previous behaviour), the broker — which runs | ||
| in the interactive session under the user's *filtered* medium-integrity | ||
| token, where Administrators is a deny-only SID — could never open the pipe, | ||
| the ConnectNamedPipe would never complete, and the loop would never | ||
| recreate the instance with the correct console-user DACL. It would hang | ||
| until a manual service restart. INTERACTIVE is present in every | ||
| interactively-logged-on token (filtered or not), so the boot-time pipe is | ||
| reachable by the interactive user as soon as they log in; once a console | ||
| user is resolved, later instances tighten to SYSTEM + that specific SID. | ||
| The privileged operations behind the pipe are policy-gated regardless. | ||
| Never falls back to a NULL DACL — that was the original mistake. | ||
|
|
||
| Raising would prevent the service from starting; instead, on failure we | ||
| return a SECURITY_ATTRIBUTES with a *deny-all* DACL so the pipe is created | ||
| but unreachable, making the failure obvious in logs rather than silent. | ||
| """ | ||
| if not _WIN32_AVAILABLE: | ||
| return None | ||
|
|
||
| import pywintypes | ||
|
|
||
| try: | ||
| # SYSTEM SID — always allowed; the service runs as SYSTEM. | ||
| sid_system = win32security.CreateWellKnownSid( | ||
| getattr(win32security, _SID_TYPE_SYSTEM), None | ||
| ) | ||
|
|
||
| # Console user SID (if anyone is logged in); else fall back to the | ||
| # INTERACTIVE group so the pipe is still reachable by the interactive | ||
| # user once they log in (see _build_pipe_sa docstring for why Admins | ||
| # would deadlock the boot-time instance). | ||
| sid_user = _console_user_sid() | ||
| if sid_user is None: | ||
| sid_user = win32security.CreateWellKnownSid( | ||
| getattr(win32security, _SID_TYPE_INTERACTIVE), None | ||
| ) | ||
| logger.info("Pipe DACL fallback: SYSTEM + NT AUTHORITY\\INTERACTIVE") | ||
| else: | ||
| logger.info( | ||
| "Pipe DACL: SYSTEM + console user %s", | ||
| win32security.ConvertSidToStringSid(sid_user), | ||
| ) | ||
|
|
||
| dacl = win32security.ACL() | ||
| dacl.AddAccessAllowedAce( | ||
| win32security.ACL_REVISION, _FILE_ALL_ACCESS, sid_system | ||
| ) | ||
| dacl.AddAccessAllowedAce( | ||
| win32security.ACL_REVISION, _FILE_ALL_ACCESS, sid_user | ||
| ) |
There was a problem hiding this comment.
1. Pipe allows interactive clients 📎 Requirement gap ⛨ Security
When the console user SID cannot be resolved (e.g., at boot), the named pipe server falls back to granting full access to NT AUTHORITY\INTERACTIVE and relies on that DACL alone for authorization, which is broader than the intended console user/session binding. Because privileged work is executed in the active console session, this weak binding enables same-session/process hijacking and a cross-session authorization mismatch where other interactive logons may connect and trigger actions against the console user’s desktop (notably if policy allows auto-click).
Agent Prompt
## Issue description
The SYSTEM service named pipe is not strongly bound to the intended interactive console user/session: when no console user SID can be resolved (common at boot), `_build_pipe_sa()` falls back to granting `FILE_ALL_ACCESS` to `NT AUTHORITY\\INTERACTIVE`, and the server performs no additional per-connection/per-request verification beyond the pipe DACL. Because requests are executed against the *active console session* regardless of which client connected, this creates a cross-session authorization mismatch and enables other interactive processes/sessions (or same-session hijackers) to trigger SYSTEM-backed actions against the console user’s desktop.
## Issue Context
Compliance (PR Compliance ID 222817) requires broker↔service IPC to enforce strong binding to the active interactive console user/session and reject other processes/sessions. The current boot-time fallback appears intended to avoid deadlock before any user logs in, but it broadens access to a SYSTEM-backed IPC channel that can drive secure-desktop/console-session operations; policy gating may exist as defense-in-depth, but it should not be the sole authorization control.
## Fix Focus Areas
- src/windows_mcp/service/host.py[103-207]
- src/windows_mcp/service/host.py[304-355]
- src/windows_mcp/service/secure_desktop.py[1112-1144]
## Recommended change
- Add **per-connection / per-request** authorization by impersonating the named-pipe client (e.g., `ImpersonateNamedPipeClient`) and verifying the client token’s user SID (and ideally session ID) matches the active console user SID/session.
- If no console user is active, consider creating a pipe instance that is SYSTEM-only (or deny-all) and periodically recreating/rebinding the pipe to pick up the console SID once available, instead of using `INTERACTIVE`.
- Keep any existing policy gating as defense-in-depth, but do not rely on it as the only protection against cross-session callers.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+690
to
+697
| # UAC routing: consent.exe runs at System integrity. UIPI blocks | ||
| # mouse input from medium-integrity processes, so a normal uia.Click | ||
| # at the dialog's Yes/No coords silently fails to dismiss UAC. The | ||
| # SYSTEM host service has a UIAccess-tokened user-session worker | ||
| # that CAN invoke across integrity; route the click there if the | ||
| # target pixel is owned by consent.exe. | ||
| if _is_consent_target(x, y) and _route_click_through_host(x, y): | ||
| return |
There was a problem hiding this comment.
5. Policy denial masked click 🐞 Bug ≡ Correctness
Desktop.click() routes consent.exe clicks through the host service but suppresses host errors (including "policy denied") and then falls back to a local click, while the Click tool always returns a success message. This hides policy enforcement from callers and can make agents believe a UAC prompt was dismissed when it was not.
Agent Prompt
## Issue description
UAC clicks that are routed through the host service can be denied by policy (host returns an error), but `_route_click_through_host()` catches the exception and returns `False`, causing `Desktop.click()` to fall back to a local `uia.Click()` and the `Click` tool to still report success. This breaks the contract described in the PR ("refused with a policy denied error") and makes automation unreliable.
## Issue Context
- Host service enforces consent policy and returns an error response on denial.
- Pipe client converts `Response.error` into a raised exception.
- Desktop click routing currently swallows that exception and falls back.
## Fix Focus Areas
- src/windows_mcp/desktop/service.py[42-83]
- src/windows_mcp/desktop/service.py[681-712]
- src/windows_mcp/tools/input.py[39-57]
- src/windows_mcp/service/pipe.py[90-118]
- src/windows_mcp/service/host.py[283-290]
## Recommended change
- In `_route_click_through_host`, do **not** swallow `RuntimeError` that indicates a policy denial (e.g., message contains `"policy denied"`); instead raise a clear exception.
- In `Desktop.click`, if `_is_consent_target(x,y)` is true and host routing fails due to policy denial, abort the click (don’t fall back to local) so the tool call surfaces the failure.
- Optionally adjust `Click` tool to return/raise an error when the underlying click was blocked/failed for consent.exe targets.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
bensheed
pushed a commit
to bensheed/Windows-MCP
that referenced
this pull request
Jul 7, 2026
Addresses qodo-code-review findings on PR CursorTouch#309: CursorTouch#2 (Bug/Correctness) "Policy denial masked click": Desktop.click routed consent.exe clicks through the host, swallowed the host's "policy denied" error, fell back to a local (UIPI-blocked, no-op) click, and the Click tool still reported success — so under policy=block the agent was told UAC was dismissed when the policy had actually refused it. Now the pipe client raises a distinct HostPolicyDenied; _route_click_through_host propagates it instead of falling back; and the Click tool returns an explicit "refused by policy" message so a human stays in the loop. CursorTouch#3/CursorTouch#4 (Rule): add type hints and a Google-style docstring to tools/uac.py register(). CursorTouch#5 (Rule): double-quote the secure-desktop config-writer block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ubSxdAaDkQxYJ17U2zuDU
Member
|
Can you pls show a demo video of the working |
Author
Yeah is there anything specific you would like to see? |
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
3 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.
Closes #236.
Today, every admin-elevation prompt is a hard stop for an automated agent: UAC
draws
consent.exeon the isolated Secure Desktop (the Winlogon desktop),which no third-party process can read or click. The agent goes blind and the
run halts.
This PR adds an opt-in service mode that lets the agent see the UAC
dialog as a UIA tree and click Yes/No through the normal
Clicktool, plus anew
WaitForUACPrompttool to block until a prompt appears. It is installedexplicitly, gated behind elevation, policy-controlled, and fully reversible.
Verified working on Windows 11 25H2 (build 10.0.26200): a triggered
elevation is detected, returned to the agent as a dialog tree with an invokable
Yes button, and dismissed by a
Click.How the feature works
Two moving parts:
1. A registry policy that routes UAC off the Secure Desktop
windows-mcp service secure-desktop installwritesHKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System:PromptOnSecureDesktop = 0ConsentPromptBehaviorAdmin = 4("Prompt for consent")With both set, UAC still fires for every elevation and the user still sees a
consent dialog — but it renders on the user's Default desktop instead of
switching to the isolated Winlogon desktop.
consent.exebecomes a top-levelwindow in the interactive session, which is the only configuration in which any
third-party process can reach it.
2. A LocalSystem helper service + user-session worker
Because a plain MCP server runs unprivileged and Session 0 isolation stops a
service from enumerating user-session windows, the work is split across three
processes connected by a named pipe:
requests over the pipe.
WindowsMCPHost— a LocalSystem service. Detectsconsent.exe, enforcesthe consent policy, and spawns the worker.
session with a UIAccess token (
TokenUIAccess = 1). This is whatactually reads and clicks the dialog.
Reading the dialog: even on the Default desktop,
consent.exeruns at Systemintegrity, so a normal UIA tree walk (
GetRootElement→ descend) does notreturn it. The worker uses cross-integrity strategies instead — locating the
consent.exeHWND viaEnumWindowsand binding to it withElementFromHandle,or
GetFocusedElement+ walk-up-to-root. It tries several in sequence and takesthe first that yields the dialog.
Clicking Yes/No:
consent.exerenders its buttons as XAML that the UIA walkercan't descend into (no invokable child element), so the worker synthesizes
the Yes/No node coordinates from the dialog's bounding box and clicks physically
with
SendInput. The worker's UIAccess token exempts thatSendInputfrom UIPI,so the click lands on the higher-integrity dialog.
Using it
Install (run elevated):
This registers the
WindowsMCPHostLocalSystem service (auto-start), writes theconsent policy, and writes the UAC registry values above.
Seeing the dialog vs. clicking it — two separate things
WaitForUACPromptreturns thefull dialog: its UIA tree, the verified publisher (when detectable), and
synthesized, clickable Yes and No nodes. The agent — and you,
directing it — always have complete visibility into what is asking to elevate.
decides whether the agent may click at all. It does not decide Yes vs.
No — that is always your call (see "Driving Yes / No" below).
Consent policies
block(default)policy deniederror, so a human has to physically click Yes or No. Keeps a person in the loop on every elevation.allow_with_matchallow_allDriving Yes / No
When the policy permits a click (
allow_all, orallow_with_matchon a trustedpublisher), you choose Yes or No in plain language and the agent acts on it —
tell it "approve this" and it clicks the Yes node; "deny it" and it clicks
No. There is no dedicated approve/deny tool: the agent just uses the normal
Clickon the Yes or No coordinates thatWaitForUACPrompthanded back.Whether it may click is the policy's job; which button it clicks is your
instruction.
So the end-to-end loop is: the agent triggers something that needs elevation →
calls
WaitForUACPrompt(blocks until the prompt appears, returns the dialog +Yes/No nodes + publisher) → you tell it to approve or deny → it
Clicks thematching node. Under
block, that click comes backpolicy deniedand a humanclicks instead.
Uninstall restores the stock secure posture (
PromptOnSecureDesktop = 1,ConsentPromptBehaviorAdmin = 5) and removes the service:What worked, and why
PromptOnSecureDesktop = 0+ConsentPromptBehaviorAdmin = 4. This is the linchpin. On the Defaultdesktop,
consent.exeis an ordinary top-level window a same-session workercan bind to.
CPB = 4specifically matters (see below).capability that actually matters here —
SendInputthat crosses UIPI integritylevels — so a synthesized physical click reaches the System-integrity dialog.
Splitting broker/service/worker sidesteps Session 0 isolation: the privileged
service can't enumerate user windows itself, so it delegates to the worker.
ElementFromHandle,GetFocusedElement+walk-up). These bind to the
consent.exeHWND directly and cross theintegrity boundary, where the ordinary
GetRootElementtree walk returnsnothing for a System-integrity window.
consent.exe's XAMLbuttons expose no descendable invoke target, so computing their positions from
the dialog rectangle and clicking via
SendInputis what makes "click Yes"work at all.
What did not work, and why
Every attempt to reach UAC while it stays on the Secure Desktop failed on
Win 11 25H2. These dead-ends were explored and then removed from the codebase;
they're recorded here so the next person can skip them:
EnumDesktopWindows,RawViewWalkerfrom the desktop root,GetForegroundWindow,ElementFromPoint,GetGUIThreadInfoperconsent.exethread). All returnedempty or only Default-desktop windows. Why: Session 0 isolation plus
Winlogon's tight DACL, and the OS filters the secure desktop's window list for
callers that aren't TrustedInstaller/Winlogon — even after a SYSTEM broker
loosens the DACL and hands the worker a granted handle.
FocusChanged/WindowOpened/StructureChanged) to catchconsent.exeon the secure desktop. Registeredfine, never fired. Why: UIAccess grants cross-UIPI
SendInput, not theaccessibility-event trust that Narrator/Magnifier rely on (kernel-baked input
callbacks / an undocumented Winlogon trust relationship a third-party process
can't replicate).
BitBltscreen-capture of the secure desktop (then scan for theblue consent-button pixels). Returned an all-black frame. Why: the
Win 10 1809+/Win 11 anti-screenscrape hardening renders the secure desktop to
a separate GPU surface GDI capture cannot touch.
PromptOnSecureDesktop = 0+ConsentPromptBehaviorAdmin = 5. Both writes stuck and read back correct,but Win 11 25H2 routed UAC to Winlogon anyway. Why: this build honours the
secure-desktop-disable at the documentation layer but ignores it at the
dispatch layer for
CPB = 5(Microsoft's modern default, which the dispatchlayer still treats as "secure desktop"). Only
CPB = 4is actually honoured —which is why the shipped install uses
4, and uninstall restores5.The net effect on this branch: the shipped code is only the path that works
(policy routing + UIAccess worker + cross-integrity read + synthesized-button
click); the cross-desktop dead-ends and an unused speculative UIA write-API were
removed so the diff is the feature and nothing else.
Security trade-off
Moving UAC off the secure desktop weakens exactly one protection: a process
already running in the user's session can now draw a look-alike dialog and
capture clicks. Everything else is intact —
EnableLUA = 1(the admin token isstill split, every elevation still prompts), the dialog still shows publisher +
path + Yes/No, nothing auto-elevates without a click, and malware still can't
programmatically click a real UAC dialog without a UIAccess (signed, trusted-path)
binary of its own.
It is therefore opt-in, elevation-gated, and reversible: the machine-wide
change begins and ends with the user running
install/uninstall, and thedefault policy (
block) keeps a human on the click even when the dialog isvisible. Appropriate for a machine dedicated to agent automation; not something
to flip on a general-purpose desktop.
Testing
Verified end-to-end on Windows 11 25H2 with the service installed and the
allow_allpolicy: a triggered elevation is (1) detected byWaitForUACPrompt,which returns the dialog as a non-empty UIA tree, (2) reports the configured
policy, (3) exposes an invokable Yes node, and (4) is dismissed by a
Clickon that node (a follow-up
WaitForUACPromptthen returnsfired: false).The
blockandallow_with_matchpolicies were checked by reviewing theenforcement path rather than a live run. No automated tests are included in
this PR for the consent policy or the UIA path, and the existing test suite is
unchanged — dedicated tests can be added as a follow-up.