mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-20 05:57:08 +00:00
fix: guard headed focus with full process identity at every point of use
This commit is contained in:
parent
f55f34cf09
commit
dbacba025c
3 changed files with 88 additions and 24 deletions
|
|
@ -68,10 +68,7 @@ impl<'a> WindowIdentityEvidence<'a> {
|
|||
/// application's tree and every check downstream - which re-verifies the
|
||||
/// same stored pid and token - would agree it was the right one.
|
||||
pub(crate) fn verify_stored(&self) -> Result<(), AdapterError> {
|
||||
if live_window_owner(self.handle) != Some(self.pid) {
|
||||
return Err(window_identity_mismatch(self.handle));
|
||||
}
|
||||
if !process_identity::matches_instance(self.pid, self.process_instance)? {
|
||||
if !self.owns_handle_now()? {
|
||||
return Err(window_identity_mismatch(self.handle));
|
||||
}
|
||||
let live = live_window_title(self.handle);
|
||||
|
|
@ -83,6 +80,27 @@ impl<'a> WindowIdentityEvidence<'a> {
|
|||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether the handle is owned, at this instant, by the stored process
|
||||
/// *instance*: the pid that owns the handle right now, and that pid still
|
||||
/// running the generation the evidence was taken from.
|
||||
///
|
||||
/// Both terms are load-bearing and neither substitutes for the other. The
|
||||
/// owner read alone cannot see a process that exited and had its pid
|
||||
/// handed to something else, because Windows recycles pids freely; the
|
||||
/// generation token alone cannot see a handle that passed to a different
|
||||
/// window of a still-running process. A caller that re-checks only the pid
|
||||
/// at the point of use is therefore weaker than the check that admitted
|
||||
/// the window, and a replacement that inherited both the recycled pid and
|
||||
/// the recycled handle would satisfy it. This predicate exists so the
|
||||
/// admission check and every point-of-use guard are the same test by
|
||||
/// construction rather than by convention.
|
||||
pub(crate) fn owns_handle_now(&self) -> Result<bool, AdapterError> {
|
||||
if live_window_owner(self.handle) != Some(self.pid) {
|
||||
return Ok(false);
|
||||
}
|
||||
process_identity::matches_instance(self.pid, self.process_instance)
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads the process that owns a window handle right now - the fact that
|
||||
|
|
|
|||
|
|
@ -1,10 +1,9 @@
|
|||
use agent_desktop_core::{
|
||||
AdapterError, Deadline, DeliverySemantics, ErrorCode, InteractionLease, ProcessId, WindowInfo,
|
||||
WindowState,
|
||||
AdapterError, Deadline, DeliverySemantics, ErrorCode, InteractionLease, WindowInfo, WindowState,
|
||||
};
|
||||
|
||||
use super::window_enum::enumerate_top_level;
|
||||
use super::window_identity::{WindowIdentityEvidence, live_window_owner, live_window_title};
|
||||
use super::window_identity::{WindowIdentityEvidence, live_window_title};
|
||||
use super::window_ops::{is_foreground_window, parse_handle, passes_filter};
|
||||
|
||||
/// Resolves a live window by `WindowInfo.id`, corroborating pid and process
|
||||
|
|
@ -75,12 +74,12 @@ pub(crate) fn focus_window(win: &WindowInfo, lease: &InteractionLease) -> Result
|
|||
));
|
||||
};
|
||||
evidence.verify_stored()?;
|
||||
restore_if_iconic(handle, win.pid)?;
|
||||
if is_owned_foreground(handle, win.pid) {
|
||||
restore_if_iconic(handle, &evidence)?;
|
||||
if is_owned_foreground(handle, &evidence) {
|
||||
return Ok(());
|
||||
}
|
||||
bring_to_foreground(handle, win.pid)?;
|
||||
if is_owned_foreground(handle, win.pid) {
|
||||
bring_to_foreground(handle, &evidence)?;
|
||||
if is_owned_foreground(handle, &evidence) {
|
||||
return Ok(());
|
||||
}
|
||||
Err(AdapterError::new(
|
||||
|
|
@ -146,29 +145,41 @@ fn window_exists(_handle: super::window_enum::WindowHandle) -> bool {
|
|||
}
|
||||
|
||||
/// Whether the handle is the foreground window **and** is still owned by the
|
||||
/// expected process. Handle equality alone would accept a recycled HWND, so
|
||||
/// the ownership term is what makes this a safe success predicate.
|
||||
/// stored process instance.
|
||||
///
|
||||
/// Handle equality alone would accept a recycled HWND; pid equality alone
|
||||
/// would accept a replacement process that inherited a recycled pid. The
|
||||
/// success predicate therefore asks the same full-identity question the
|
||||
/// admission check asks, and an unreadable answer is not a success — a
|
||||
/// generation read that fails leaves the window unproven, so this reports
|
||||
/// false and the caller returns not-delivered.
|
||||
#[cfg(target_os = "windows")]
|
||||
fn is_owned_foreground(handle: super::window_enum::WindowHandle, expected: ProcessId) -> bool {
|
||||
is_foreground_window(handle) && live_window_owner(handle) == Some(expected)
|
||||
fn is_owned_foreground(
|
||||
handle: super::window_enum::WindowHandle,
|
||||
evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> bool {
|
||||
is_foreground_window(handle) && evidence.owns_handle_now().unwrap_or(false)
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
fn is_owned_foreground(_handle: super::window_enum::WindowHandle, _expected: ProcessId) -> bool {
|
||||
fn is_owned_foreground(
|
||||
_handle: super::window_enum::WindowHandle,
|
||||
_evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn restore_if_iconic(
|
||||
handle: super::window_enum::WindowHandle,
|
||||
expected: ProcessId,
|
||||
evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> Result<(), AdapterError> {
|
||||
use windows_sys::Win32::UI::WindowsAndMessaging::{IsIconic, SW_RESTORE, ShowWindow};
|
||||
unsafe {
|
||||
if IsIconic(handle) == 0 {
|
||||
return Ok(());
|
||||
}
|
||||
if live_window_owner(handle) != Some(expected) {
|
||||
if !evidence.owns_handle_now()? {
|
||||
return Err(recycled_before_foreground());
|
||||
}
|
||||
ShowWindow(handle, SW_RESTORE);
|
||||
|
|
@ -179,7 +190,7 @@ fn restore_if_iconic(
|
|||
#[cfg(not(target_os = "windows"))]
|
||||
fn restore_if_iconic(
|
||||
_handle: super::window_enum::WindowHandle,
|
||||
_expected: ProcessId,
|
||||
_evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> Result<(), AdapterError> {
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -187,7 +198,7 @@ fn restore_if_iconic(
|
|||
#[cfg(target_os = "windows")]
|
||||
fn bring_to_foreground(
|
||||
handle: super::window_enum::WindowHandle,
|
||||
expected: ProcessId,
|
||||
evidence: &WindowIdentityEvidence<'_>,
|
||||
) -> Result<(), AdapterError> {
|
||||
use windows_sys::Win32::System::Threading::{AttachThreadInput, GetCurrentThreadId};
|
||||
use windows_sys::Win32::UI::WindowsAndMessaging::{
|
||||
|
|
@ -196,7 +207,7 @@ fn bring_to_foreground(
|
|||
};
|
||||
|
||||
unsafe {
|
||||
if live_window_owner(handle) != Some(expected) {
|
||||
if !evidence.owns_handle_now()? {
|
||||
return Err(recycled_before_foreground());
|
||||
}
|
||||
if IsWindowVisible(handle) == 0 {
|
||||
|
|
@ -204,7 +215,7 @@ fn bring_to_foreground(
|
|||
}
|
||||
let mut target_pid = 0u32;
|
||||
let target_tid = GetWindowThreadProcessId(handle, &mut target_pid);
|
||||
if target_tid == 0 || ProcessId::from(target_pid) != expected {
|
||||
if target_tid == 0 || !evidence.owns_handle_now()? {
|
||||
return Err(recycled_before_foreground());
|
||||
}
|
||||
let foreground = GetForegroundWindow();
|
||||
|
|
@ -221,7 +232,7 @@ fn bring_to_foreground(
|
|||
let attached_target = target_tid != 0
|
||||
&& target_tid != current_tid
|
||||
&& AttachThreadInput(current_tid, target_tid, 1) != 0;
|
||||
let still_owned = live_window_owner(handle) == Some(expected);
|
||||
let still_owned = evidence.owns_handle_now().unwrap_or(false);
|
||||
if still_owned {
|
||||
let _ = SetForegroundWindow(handle);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -112,6 +112,39 @@ fn focus_window_refuses_a_destroyed_handle_before_any_window_write() {
|
|||
/// outcome: whether a lane can take the foreground at all depends on the
|
||||
/// desktop it runs on, so this asserts the implication rather than the
|
||||
/// result. `Ok` must mean the foreground window is the target **and** is
|
||||
/// Every point-of-use guard asks the full-identity question, not the pid
|
||||
/// alone. A pid that Windows recycled to a replacement process satisfies pid
|
||||
/// equality while the generation token does not, so a guard weakened to pid
|
||||
/// equality would admit exactly the window the admission check refused. This
|
||||
/// drives the shared predicate with a live fixture and with a generation
|
||||
/// token that cannot match, and the second case fails if any guard is
|
||||
/// reverted to comparing the owning pid.
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn a_recycled_generation_is_refused_even_when_the_owning_pid_matches() {
|
||||
let (_fixture, expected) = listed_fixture_window();
|
||||
let handle = parse_handle(&expected.id);
|
||||
|
||||
let genuine = WindowIdentityEvidence::from_info(handle, &expected).expect("stored evidence");
|
||||
assert!(
|
||||
genuine.owns_handle_now().expect("owner read"),
|
||||
"the live fixture window must satisfy its own stored evidence"
|
||||
);
|
||||
|
||||
let mut impostor_info = expected.clone();
|
||||
impostor_info.process_instance = Some("windows-proc-v1:0:0".into());
|
||||
let impostor =
|
||||
WindowIdentityEvidence::from_info(handle, &impostor_info).expect("stored evidence");
|
||||
assert!(
|
||||
!impostor.owns_handle_now().expect("owner read"),
|
||||
"a stale generation token must be refused even though the owning pid still matches"
|
||||
);
|
||||
assert!(
|
||||
!is_owned_foreground(handle, &impostor),
|
||||
"the success predicate must refuse a matching pid on a stale generation"
|
||||
);
|
||||
}
|
||||
|
||||
/// still owned by the expected process; a lane that cannot focus must say
|
||||
/// so as a not-delivered failure. The forbidden state is `Ok` while some
|
||||
/// other process owns the foreground.
|
||||
|
|
@ -123,8 +156,10 @@ fn focus_window_reports_ok_only_when_the_expected_process_owns_the_foreground()
|
|||
match focus_window(&expected, &lease) {
|
||||
Ok(()) => {
|
||||
let handle = parse_handle(&expected.id);
|
||||
let evidence =
|
||||
WindowIdentityEvidence::from_info(handle, &expected).expect("stored evidence");
|
||||
assert!(
|
||||
is_owned_foreground(handle, expected.pid),
|
||||
is_owned_foreground(handle, &evidence),
|
||||
"focus_window returned Ok while the foreground was not the owned target"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue