fix: harden Windows window target discovery

.github/workflows/windows-targets.yml

@@ -0,0 +1,57 @@
+name: Windows Target Discovery
+
+on:
+  pull_request:
+    paths:
+      - "crates/scap-targets/**"
+      - "crates/recording/src/sources/screen_capture/**"
+      - "apps/cli/src/targets.rs"
+      - "apps/cli/src/record.rs"
+      - "Cargo.toml"
+      - "Cargo.lock"
+      - ".github/workflows/windows-targets.yml"
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: windows-targets-${{ github.head_ref || github.ref_name }}
+  cancel-in-progress: true
+
+jobs:
+  windows-targets:
+    name: Native Windows window discovery
+    runs-on: windows-2022
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262
+        with:
+          fetch-depth: 2
+      - uses: dtolnay/rust-toolchain@688313b0823df1393bcebb1b4add0438a6d36884
+        with:
+          components: clippy
+      - uses: ./.github/actions/setup-rust-cache
+        with:
+          target: x86_64-pc-windows-msvc
+      - run: cargo test --locked -p scap-targets --lib --test windows_targets -- --nocapture
+      - name: Compare discovery against the PR base
+        if: github.event_name == 'pull_request'
+        shell: pwsh
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+        run: |
+          $source = "crates/scap-targets/src/platform/win.rs"
+          $patchedSource = [System.IO.File]::ReadAllBytes($source)
+          try {
+            $baseline = git show "$($env:BASE_SHA):$source"
+            if ($LASTEXITCODE -ne 0) { throw "Could not load base Windows implementation" }
+            $baseline | Set-Content -Encoding utf8 $source
+            cargo test --locked -p scap-targets --test windows_targets -- --nocapture
+            $baselineExit = $LASTEXITCODE
+            "Base implementation discovery test exit code: $baselineExit (0 means this fixture already passed on the base)." | Add-Content $env:GITHUB_STEP_SUMMARY
+          } finally {
+            [System.IO.File]::WriteAllBytes($source, $patchedSource)
+          }
+          exit 0
+      - run: cargo clippy --locked -p scap-targets --all-targets -- -D warnings

crates/scap-targets/src/platform/win.rs

@@ -1,4 +1,10 @@
-use std::{ffi::OsString, mem, os::windows::ffi::OsStringExt, path::PathBuf, str::FromStr};
+use std::{
+    ffi::OsString,
+    mem,
+    os::windows::ffi::OsStringExt,
+    path::{Path, PathBuf},
+    str::FromStr,
+};
 use tracing::error;
 use windows::{
     Graphics::Capture::GraphicsCaptureItem,
@@ -36,14 +42,13 @@ use windows::{
                 SHGetFileInfoW,
             },
             WindowsAndMessaging::{
-                DI_FLAGS, DestroyIcon, DrawIconEx, EnumChildWindows, EnumWindows, GCLP_HICON,
-                GW_HWNDNEXT, GWL_EXSTYLE, GWL_STYLE, GetClassLongPtrW, GetClassNameW,
-                GetClientRect, GetCursorPos, GetDesktopWindow, GetIconInfo,
-                GetLayeredWindowAttributes, GetWindow, GetWindowLongPtrW, GetWindowLongW,
-                GetWindowRect, GetWindowTextLengthW, GetWindowTextW, GetWindowThreadProcessId,
-                HICON, ICONINFO, IsIconic, IsWindowVisible, PrivateExtractIconsW, SendMessageW,
-                WM_GETICON, WS_CHILD, WS_EX_LAYERED, WS_EX_TOOLWINDOW, WS_EX_TOPMOST,
-                WS_EX_TRANSPARENT, WindowFromPoint,
+                DI_FLAGS, DestroyIcon, DrawIconEx, EnumWindows, GCLP_HICON, GW_HWNDNEXT,
+                GWL_EXSTYLE, GWL_STYLE, GetClassLongPtrW, GetClassNameW, GetClientRect,
+                GetCursorPos, GetIconInfo, GetLayeredWindowAttributes, GetWindow,
+                GetWindowLongPtrW, GetWindowLongW, GetWindowRect, GetWindowTextLengthW,
+                GetWindowTextW, GetWindowThreadProcessId, HICON, ICONINFO, IsIconic,
+                IsWindowVisible, PrivateExtractIconsW, SendMessageW, WM_GETICON, WS_CHILD,
+                WS_EX_LAYERED, WS_EX_TOOLWINDOW, WS_EX_TOPMOST, WS_EX_TRANSPARENT, WindowFromPoint,
             },
         },
     },
@@ -329,11 +334,12 @@ impl WindowImpl {
         };
 
         unsafe {
-            let _ = EnumChildWindows(
-                Some(GetDesktopWindow()),
+            if let Err(error) = EnumWindows(
                 Some(enum_windows_proc),
                 LPARAM(std::ptr::addr_of_mut!(context) as isize),
-            );
+            ) {
+                error!(%error, "Failed to enumerate top-level windows");
+            }
         }
 
         context.list
@@ -1165,8 +1171,7 @@ impl WindowImpl {
         }
 
         if let Ok(exe_path) = unsafe { pid_to_exe_path(id) }
-            && let Some(exe_name) = exe_path.file_name().and_then(|n| n.to_str())
-            && IGNORED_EXES.contains(&&*exe_name.to_lowercase())
+            && is_ignored_executable(&exe_path)
         {
             return false;
         }
@@ -1196,6 +1201,16 @@ impl WindowImpl {
     }
 }
 
+fn is_ignored_executable(path: &Path) -> bool {
+    path.file_stem()
+        .and_then(|name| name.to_str())
+        .is_some_and(|name| {
+            IGNORED_EXES
+                .iter()
+                .any(|ignored| name.eq_ignore_ascii_case(ignored))
+        })
+}
+
 fn is_window_valid_for_enumeration(hwnd: HWND, current_process_id: u32) -> bool {
     unsafe {
         if !IsWindowVisible(hwnd).as_bool() || IsIconic(hwnd).as_bool() {
@@ -1209,8 +1224,7 @@ fn is_window_valid_for_enumeration(hwnd: HWND, current_process_id: u32) -> bool
         }
 
         if let Ok(exe_path) = pid_to_exe_path(process_id)
-            && let Some(exe_name) = exe_path.file_name().and_then(|n| n.to_str())
-            && IGNORED_EXES.contains(&&*exe_name.to_lowercase())
+            && is_ignored_executable(&exe_path)
         {
             return false;
         }
@@ -1363,3 +1377,18 @@ unsafe fn pid_to_exe_path(pid: u32) -> Result<PathBuf, windows::core::Error> {
     let os_str = &OsString::from_wide(&lpexename[..lpdwsize as usize]);
     Ok(PathBuf::from(os_str))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn ignored_executables_match_stems_without_excluding_other_apps() {
+        for name in ["cap.exe", "CAP.EXE", "WebView2.exe", "msedgewebview2.exe"] {
+            assert!(is_ignored_executable(&Path::new(r"C:\Apps").join(name)));
+        }
+        for name in ["camoufox.exe", "firefox.exe", "inkscape.exe", "capture.exe"] {
+            assert!(!is_ignored_executable(&Path::new(r"C:\Apps").join(name)));
+        }
+    }
+}

crates/scap-targets/tests/windows_targets.rs

@@ -0,0 +1,189 @@
+#![cfg(target_os = "windows")]
+
+use scap_targets::{Window, WindowId};
+use std::{
+    io::{BufRead, BufReader, Write},
+    process::{Child, Command, Stdio},
+    sync::mpsc,
+    time::Duration,
+};
+use windows::{
+    Win32::{
+        Foundation::HWND,
+        UI::{
+            HiDpi::{PROCESS_PER_MONITOR_DPI_AWARE, SetProcessDpiAwareness},
+            WindowsAndMessaging::{
+                CreateWindowExW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, PeekMessageW,
+                SW_MINIMIZE, ShowWindow, TranslateMessage, WINDOW_EX_STYLE, WINDOW_STYLE, WS_CHILD,
+                WS_EX_TOOLWINDOW, WS_OVERLAPPEDWINDOW, WS_POPUP, WS_VISIBLE,
+            },
+        },
+    },
+    core::w,
+};
+
+struct TestWindow(HWND);
+
+impl TestWindow {
+    fn new(style: WINDOW_STYLE, extended: WINDOW_EX_STYLE, parent: Option<HWND>) -> Self {
+        Self(unsafe {
+            CreateWindowExW(
+                extended,
+                w!("STATIC"),
+                w!("Cap window discovery regression"),
+                style,
+                100,
+                100,
+                320,
+                240,
+                parent,
+                None,
+                None,
+                None,
+            )
+            .expect("create native test window")
+        })
+    }
+
+    fn id(&self) -> WindowId {
+        (self.0.0 as u64).to_string().parse().unwrap()
+    }
+}
+
+impl Drop for TestWindow {
+    fn drop(&mut self) {
+        let _ = unsafe { DestroyWindow(self.0) };
+    }
+}
+
+struct FixtureProcess(Child);
+
+impl Drop for FixtureProcess {
+    fn drop(&mut self) {
+        let _ = self.0.kill();
+        let _ = self.0.wait();
+    }
+}
+
+#[test]
+fn window_fixture_process() {
+    if std::env::var_os("CAP_WINDOW_DISCOVERY_FIXTURE").is_none() {
+        return;
+    }
+
+    unsafe { SetProcessDpiAwareness(PROCESS_PER_MONITOR_DPI_AWARE) }
+        .expect("match CLI per-monitor DPI awareness");
+
+    let normal = TestWindow::new(WS_OVERLAPPEDWINDOW | WS_VISIBLE, WINDOW_EX_STYLE(0), None);
+    let popup = TestWindow::new(WS_POPUP | WS_VISIBLE, WINDOW_EX_STYLE(0), Some(normal.0));
+    let hidden = TestWindow::new(WS_OVERLAPPEDWINDOW, WINDOW_EX_STYLE(0), None);
+    let child = TestWindow::new(WS_CHILD | WS_VISIBLE, WINDOW_EX_STYLE(0), Some(normal.0));
+    let tool = TestWindow::new(WS_OVERLAPPEDWINDOW | WS_VISIBLE, WS_EX_TOOLWINDOW, None);
+    let minimized = TestWindow::new(WS_OVERLAPPEDWINDOW | WS_VISIBLE, WINDOW_EX_STYLE(0), None);
+    let _ = unsafe { ShowWindow(minimized.0, SW_MINIMIZE) };
+    println!(
+        "WINDOW_FIXTURE {} {} {} {} {} {}",
+        normal.id(),
+        popup.id(),
+        hidden.id(),
+        child.id(),
+        tool.id(),
+        minimized.id()
+    );
+    std::io::stdout().flush().unwrap();
+
+    let (stop_tx, stop_rx) = mpsc::channel();
+    std::thread::spawn(move || {
+        let mut line = String::new();
+        let _ = std::io::stdin().read_line(&mut line);
+        let _ = stop_tx.send(());
+    });
+    let started = std::time::Instant::now();
+    while started.elapsed() < Duration::from_secs(30) && stop_rx.try_recv().is_err() {
+        let mut message = MSG::default();
+        while unsafe { PeekMessageW(&mut message, None, 0, 0, PM_REMOVE) }.as_bool() {
+            let _ = unsafe { TranslateMessage(&message) };
+            unsafe { DispatchMessageW(&message) };
+        }
+        std::thread::sleep(Duration::from_millis(10));
+    }
+}
+
+#[test]
+fn discovers_foreign_top_level_windows_and_preserves_target_filters() {
+    unsafe { SetProcessDpiAwareness(PROCESS_PER_MONITOR_DPI_AWARE) }
+        .expect("match CLI per-monitor DPI awareness");
+    let mut fixture = FixtureProcess(
+        Command::new(std::env::current_exe().unwrap())
+            .args(["--exact", "window_fixture_process", "--nocapture"])
+            .env("CAP_WINDOW_DISCOVERY_FIXTURE", "1")
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .spawn()
+            .expect("start native window fixture"),
+    );
+    let output = fixture.0.stdout.take().unwrap();
+    let (ready_tx, ready_rx) = mpsc::channel();
+    std::thread::spawn(move || {
+        for line in BufReader::new(output).lines() {
+            let Ok(line) = line else { break };
+            if let Some((_, ids)) = line.split_once("WINDOW_FIXTURE ") {
+                let _ = ready_tx.send(ids.to_string());
+            }
+        }
+    });
+    let ids: Vec<WindowId> = ready_rx
+        .recv_timeout(Duration::from_secs(15))
+        .expect("native window fixture must become ready")
+        .split_whitespace()
+        .map(|id| id.parse().unwrap())
+        .collect();
+    assert_eq!(ids.len(), 6);
+    let own = TestWindow::new(WS_OVERLAPPEDWINDOW | WS_VISIBLE, WINDOW_EX_STYLE(0), None);
+    let windows = Window::list();
+    assert!(!windows.iter().any(|window| window.id() == own.id()));
+
+    for id in &ids[..2] {
+        let window = windows
+            .iter()
+            .find(|window| &window.id() == id)
+            .expect("visible top-level and owned popup windows must be discoverable");
+        assert!(window.raw_handle().is_valid());
+        assert!(window.raw_handle().is_on_screen());
+        assert!(window.name().is_some_and(|name| !name.is_empty()));
+        assert!(window.owner_name().is_some());
+        assert!(window.display().is_some());
+        assert!(window.display_relative_logical_bounds().is_some());
+        assert_eq!(
+            window.raw_handle().inner().0 as u64,
+            id.to_string().parse::<u64>().unwrap()
+        );
+        let parsed: WindowId = id.to_string().parse().unwrap();
+        assert_eq!(
+            Window::from_id(&parsed)
+                .expect("resolve listed window ID")
+                .id(),
+            *id
+        );
+    }
+
+    let eligible: Vec<_> = windows
+        .iter()
+        .filter(|window| window.raw_handle().is_valid() && window.raw_handle().is_on_screen())
+        .map(Window::id)
+        .collect();
+    for id in &ids[2..] {
+        assert!(
+            !eligible.contains(id),
+            "ineligible window {id} must stay excluded"
+        );
+    }
+    fixture
+        .0
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(b"stop\n")
+        .unwrap();
+    assert!(fixture.0.wait().unwrap().success());
+}