Creepso/agent-desktop
1
0
Fork 0
mirror of https://github.com/lahfir/agent-desktop.git synced 2026-08-05 13:40:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: refuse an unbounded uia client instead of falling back to one

Browse source 
Greptile's fourth pass, and it was right again: the fallback to
UIAutomation::new_direct() kept an unbounded path alive, so the hang
guarantee degraded silently on any build where CUIAutomation8 was missing.

That build cannot occur. CUIAutomation8 has shipped since Windows 8, the
product floor is Windows 10 1809, and it is measured present on both build
17763 and the Server 2025 CI image - the shipped-client test asserts the
connection timeout is set and passes on the Windows lane. So the fallback
bought availability nowhere and cost a safety property everywhere.

Client creation now fails with a structured error naming the reason rather
than handing back a client whose calls cannot time out. A client that cannot
be bounded is one this crate should refuse, not one it should quietly accept.

Also corrects a doc comment that still claimed construction was
new_direct()-only.
This commit is contained in:
Lahfir 2026-07-28 04:02:32 -06:00
parent 2b9fa428d7
commit 55141fe2fa
3 changed files with 46 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

65
crates/windows/src/tree/automation.rs
View file

@ -186,12 +186,13 @@ mod imp {
/// Hands out this thread's UI Automation client.
///
/// Constructed with `new_direct()` only. `new()` would call
/// `CoInitializeEx` itself: on a thread already in the MTA that returns
/// `S_FALSE` and permanently leaks one initialization count, and on any
/// STA host thread it fails outright with `RPC_E_CHANGED_MODE`. Sub-phase
/// 2.1's bootstrap owns the apartment, so this accessor asserts the
/// precondition instead of establishing it.
/// Constructed by direct `CoCreateInstance`, never through
/// `UIAutomation::new()`. `new()` would call `CoInitializeEx` itself: on a
/// thread already in the MTA that returns `S_FALSE` and permanently leaks
/// one initialization count, and on any STA host thread it fails outright
/// with `RPC_E_CHANGED_MODE`. Sub-phase 2.1's bootstrap owns the
/// apartment, so this accessor asserts the precondition instead of
/// establishing it. See `create_bounded_client` for which CLSID, and why.
pub fn automation_client() -> Result<UIAutomation, AdapterError> {
CLIENT.with(|cell| {
if let Some(client) = cell.get() {
@ -203,41 +204,51 @@ mod imp {
})
}
/// Builds a client whose calls are bounded, falling back to the crate's
/// own constructor when they cannot be.
/// Builds a client whose calls are bounded, and fails rather than hand
/// back one whose calls are not.
///
/// `UIAutomation::new_direct()` is `CoCreateInstance(&CUIAutomation, ...)`,
/// and on build 17763 that object does not support `IUIAutomation2`, so its
/// calls have no timeout at all: measured against a window that stopped
/// dispatching, `ElementFromHandle` did not return inside a 30 s watchdog.
/// and that object does not support `IUIAutomation2`, so its calls carry no
/// timeout at all: measured against a window that stopped dispatching,
/// `ElementFromHandle` did not return inside a 30 s watchdog.
/// `CUIAutomation8` exposes `SetConnectionTimeout`, and the same call then
/// returns `UIA_E_TIMEOUT` in 1.02 s.
/// returns `UIA_E_TIMEOUT` inside the bound.
///
/// This keeps every property `new_direct()` was chosen for - it is the same
/// direct `CoCreateInstance`, it never calls `CoInitializeEx`, so it works
/// inside an STA host and leaks no initialization count in a long-lived
/// process. Only the CLSID differs, and the fallback preserves the original
/// path wherever `CUIAutomation8` is unavailable.
/// process. Only the CLSID differs.
///
/// There is deliberately **no fallback to the unbounded client**. A
/// fallback would silently trade the hang guarantee for availability on a
/// platform that cannot occur: `CUIAutomation8` has shipped since
/// Windows 8, the product's floor is Windows 10 1809, and it is measured
/// present on both build 17763 and the Server 2025 CI image. A client
/// whose calls cannot be bounded is one this crate should refuse, not one
/// it should quietly accept.
fn create_bounded_client() -> Result<UIAutomation, AdapterError> {
match bounded_automation() {
Some(automation) => Ok(UIAutomation::from(automation)),
None => UIAutomation::new_direct()
.map_err(|error| uia_error(&error, "create a UI Automation client")),
}
}
fn bounded_automation() -> Option<IUIAutomation> {
let client: IUIAutomation2 =
unsafe { CoCreateInstance(&CUIAutomation8, None, CLSCTX_ALL) }.ok()?;
let client: IUIAutomation2 = unsafe { CoCreateInstance(&CUIAutomation8, None, CLSCTX_ALL) }
.map_err(|error| unbounded_client_error(error.code().0))?;
unsafe {
client
.SetConnectionTimeout(super::CONNECTION_TIMEOUT_MS)
.ok()?;
.map_err(|error| unbounded_client_error(error.code().0))?;
client
.SetTransactionTimeout(super::TRANSACTION_TIMEOUT_MS)
.ok()?;
.map_err(|error| unbounded_client_error(error.code().0))?;
}
Some(client.into())
let automation: IUIAutomation = client.into();
Ok(UIAutomation::from(automation))
}
fn unbounded_client_error(hresult: i32) -> AdapterError {
uia_failure_error(
UiaFailure::Hresult(hresult),
"create a UI Automation client whose calls are bounded",
)
.with_suggestion(
"This build does not provide CUIAutomation8; observation is refused rather than run against a client that cannot time out",
)
}
/// Resolves a top-level window handle to its UI Automation root element.

12
crates/windows/src/tree/automation_tests.rs
View file

@ -255,10 +255,9 @@ mod windows_only {
/// Why the client is built from `CUIAutomation8` rather than the CLSID
/// `UIAutomation::new_direct()` uses: the crate's object does not support
/// `IUIAutomation2` on this build, so its calls carry no timeout at all.
///
/// If this ever starts succeeding, the fallback path in
/// `create_bounded_client` becomes bounded too and this test says so.
/// `IUIAutomation2`, so its calls carry no timeout at all. That is the
/// whole reason this crate does not use it, and the reason there is no
/// fallback to it.
#[test]
fn the_crates_own_client_carries_no_timeout_which_is_why_it_is_not_used() {
use windows::Win32::UI::Accessibility::{IUIAutomation, IUIAutomation2};
@ -270,7 +269,10 @@ mod windows_only {
assert!(raw.cast::<IUIAutomation2>().is_err());
}
/// The client this crate hands out is the bounded one.
/// The client this crate hands out is the bounded one - always, with no
/// unbounded fallback. A client whose calls cannot time out is refused
/// rather than quietly accepted, so this assertion holds on every build
/// the product supports or client creation fails outright.
#[test]
fn the_shipped_client_exposes_the_timeouts_it_sets() {
use windows::Win32::UI::Accessibility::{IUIAutomation, IUIAutomation2};

2
probes/windows/FINDINGS.md
View file

@ -174,7 +174,7 @@ Captures are `14-ci-capability/captures/{session,uia-capability}-{devbox,ci}.jso
| A14-9 | `cargo test -p agent-desktop-windows --lib -- tree::properties` | uia3-com | api-contract | A14-4 established that the sibling axis cannot distinguish a dead provider from end-of-list; nothing measured what a *property* read does once the target process has exited | it does not fail either. With the host process killed and its elements retained, `GetCurrentPropertyValue` for `ClassName`, `Name` and `Value` all return `S_OK` carrying an empty `VT_BSTR`, with **no error on any of the three** - the client-side HWND proxy answers locally rather than attempting a call the dead target cannot serve | NEW-EDGE | process death is invisible on the property axis as well as the sibling axis, so neither can carry liveness. Only descent (`get_first_child`, A14-4) surfaces it. The consequence for 2.2 is a rule rather than a detection: a provider that went away must never be reported `Absent`, because `Absent` is a legitimate answer that satisfies completeness gating, and a dead target must not be able to satisfy `EvidenceRequirements` it never answered. The read path asserts that rule; it does not claim to detect death |
| A14-10 | `cargo test -p agent-desktop-windows --lib` | n/a | api-contract | 2.1's `ensure_owned_process_mta_and_dpi` guards `CoInitializeEx` behind a process-wide `OnceLock`, with its own doc-comment recording that this is sound "only because the CLI calls this once from its main thread before any COM work" | the caveat is load-bearing and 2.2 is the first consumer to reach it. `CoInitializeEx` is thread-local while the guard is process-wide, so in a multi-threaded test binary exactly one thread joins the apartment and every other thread's `CoCreateInstance` returns `CO_E_NOTINITIALIZED`; observed as 17 of 62 tree tests failing under default test parallelism and 0 of 62 with `--test-threads=1` | NEW-EDGE | no live defect - the CLI is single-threaded at bootstrap and the cdylib already uses the process-wide `CoIncrementMTAUsage` path. 2.2's tests use `ensure_hosted_library_mta_and_dpi`, which is the semantically correct primitive for threads this product does not own. Recorded because the next consumer that reaches COM from a worker thread - Phase 5's daemon, or any 2.4+ code that walks off the main thread - hits the same wall, and the failure is a confusing `CO_E_NOTINITIALIZED` from a process that did bootstrap successfully |
| A14-11 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register records that UIA has no per-element messaging timeout, that `ConnectionTimeout` and `TransactionTimeout` are not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues, and that "whether a non-pumping target produces a clean timeout or a hang is unverified, and the fixture cannot produce the condition" | the fixture **can** produce it: `CreateWindowExW` dispatches `WM_CREATE` inline, so a thread can own a live, visible, non-zero-rect window and then never dispatch again. Against that window `ElementFromHandle` produces **neither** a clean timeout nor a bounded failure - it blocks. Measured by removing the mitigation below: the resolver did not return within the test's 30 s watchdog and the case took 59.21 s to fail, against 2.01 s with it. A `Deadline` checked before and after the call cannot interrupt it, because the block is inside the call | NEW-EDGE | the question is closed: it is a hang, not a timeout. `root_from_hwnd` now asks `SendMessageTimeoutW(WM_NULL, SMTO_ABORTIFHUNG)` first, so a target that is already hung becomes a structured `APP_UNRESPONSIVE`, and asks `IsWindow` before that so a destroyed handle stays `WINDOW_NOT_FOUND` per A14-5 rather than being reported as hung. This is a mitigation, not a guarantee - a target that stops pumping between the probe and the call still blocks, and bounding that needs the call issued on a thread the caller can abandon. 2.4 owns the snapshot path that would need it |
| A14-12 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register names `ConnectionTimeout` (2 s) and `TransactionTimeout` (20 s) as UIA's own bounds and records that they are "not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues" | **they do bound it** - the obstacle was reaching them, not their effect. `uiautomation::UIAutomation::new_direct()` is `CoCreateInstance(&CUIAutomation, ...)`, and on build 17763 that object returns `E_NOINTERFACE` (`0x80004002`) for `IUIAutomation2`, where the setters live, so calls through the crate's own client carry no timeout at all. Built instead from `CUIAutomation8` with `SetConnectionTimeout(2000)`, the identical call against a window whose thread owns it but never dispatches returns `UIA_E_TIMEOUT` (`0x80131505`) in **1.02 s**; through the crate's client the same call did not return inside a 30 s watchdog and the case took 59.09 s to fail | CONTRADICTS | the resolver's remaining hang was closable and is closed. The client is constructed from `CUIAutomation8` by direct `CoCreateInstance`, which keeps every property `new_direct()` was chosen for - it never calls `CoInitializeEx`, so it works inside an STA host and leaks no initialization count in a long-lived process - and differs only in CLSID, with a fallback to `new_direct()` wherever `CUIAutomation8` is unavailable. **This is a deviation from the Definition of Done's "constructed with `new_direct()` only" and is flagged for the owner rather than assumed**; KTD1's own prohibition is on `UIAutomation::new()`, which is still never called, and both of KTD1's stated reasons are preserved. A test pins that the crate's client lacks the interface, so if that changes the fallback becomes bounded too and says so |
| A14-12 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register names `ConnectionTimeout` (2 s) and `TransactionTimeout` (20 s) as UIA's own bounds and records that they are "not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues" | **they do bound it** - the obstacle was reaching them, not their effect. `uiautomation::UIAutomation::new_direct()` is `CoCreateInstance(&CUIAutomation, ...)`, and on build 17763 that object returns `E_NOINTERFACE` (`0x80004002`) for `IUIAutomation2`, where the setters live, so calls through the crate's own client carry no timeout at all. Built instead from `CUIAutomation8` with `SetConnectionTimeout(2000)`, the identical call against a window whose thread owns it but never dispatches returns `UIA_E_TIMEOUT` (`0x80131505`) in **1.02 s**; through the crate's client the same call did not return inside a 30 s watchdog and the case took 59.09 s to fail | CONTRADICTS | the resolver's remaining hang was closable and is closed. The client is constructed from `CUIAutomation8` by direct `CoCreateInstance`, which keeps every property `new_direct()` was chosen for - it never calls `CoInitializeEx`, so it works inside an STA host and leaks no initialization count in a long-lived process - and differs only in CLSID. There is deliberately **no fallback** to the unbounded client: a fallback would trade the hang guarantee for availability on a platform that cannot occur, since `CUIAutomation8` has shipped since Windows 8, the product floor is Windows 10 1809, and it is measured present on both build 17763 and the Server 2025 CI image. A client whose calls cannot be bounded is refused outright. **This is a deviation from the Definition of Done's "constructed with `new_direct()` only" and is flagged for the owner rather than assumed**; KTD1's own prohibition is on `UIAutomation::new()`, which is still never called, and both of KTD1's stated reasons are preserved. A test pins that the crate's client lacks the interface, which is the whole reason it is not used |
## Session evidence (R6)