From d5c21af4e5f9e5d175e2226df00c72a29d9b68e4 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:01:33 +0200 Subject: [PATCH 01/10] refactor: confine desktop and setup helpers --- src/about_window/clipboard.rs | 116 +++++++++++-------- src/about_window/state.rs | 27 +++-- src/app/mod.rs | 30 ++++- src/clipboard_text.rs | 14 ++- src/daemon/setup.rs | 61 +++++++++- src/daemon/tray/helpers.rs | 40 +++---- src/desktop_open.rs | 109 ++++++++++++++++++ src/input/state/core/utility/launcher.rs | 30 +---- src/lib.rs | 1 + src/process_broker/manifest.rs | 51 ++++++++- src/process_broker/tests.rs | 138 ++++++++++++++++++++++- src/process_broker/wire.rs | 1 + src/update_check/mod.rs | 1 + 13 files changed, 500 insertions(+), 119 deletions(-) create mode 100644 src/desktop_open.rs diff --git a/src/about_window/clipboard.rs b/src/about_window/clipboard.rs index 7debe6b2b..3c4951be9 100644 --- a/src/about_window/clipboard.rs +++ b/src/about_window/clipboard.rs @@ -1,28 +1,28 @@ -use anyhow::{Context, Result}; +use anyhow::Result; use log::warn; -pub(super) fn open_url(url: &str) { - let opener = if cfg!(target_os = "macos") { - "open" - } else if cfg!(target_os = "windows") { - "cmd" - } else { - "xdg-open" - }; - - let mut cmd = std::process::Command::new(opener); - if cfg!(target_os = "windows") { - cmd.args(["/C", "start", ""]).arg(url); - } else { - cmd.arg(url); - } - cmd.stdin(std::process::Stdio::null()); - cmd.stdout(std::process::Stdio::null()); - cmd.stderr(std::process::Stdio::null()); +pub(super) fn open_url(url: &str) -> Result<()> { + open_url_with(url, |invocation| { + // This runs from a Wayland protocol handler. Match the overlay + // launchers by refusing a contended broker exchange instead of + // stalling input and redraw behind another helper. + crate::process_broker::current()?.try_spawn( + crate::process_broker::HelperKind::DesktopOpen, + crate::process_broker::HelperLifetime::DetachedAfterExec, + invocation.program(), + invocation.arguments(), + Vec::new(), + )?; + Ok(()) + }) +} - if let Err(err) = cmd.spawn() { - warn!("Failed to open URL {}: {}", url, err); - } +fn open_url_with( + url: &str, + spawn: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> Result<()>, +) -> Result<()> { + let invocation = crate::desktop_open::about_url(url)?; + spawn(&invocation) } pub(super) fn copy_text_to_clipboard(text: &str) { @@ -33,7 +33,7 @@ pub(super) fn copy_text_to_clipboard(text: &str) { let text = text.to_string(); std::thread::spawn(move || { if let Err(err) = copy_text_with_command(&text, copy_text_via_command) { - warn!("Failed to copy commit id to clipboard: {}", err); + warn!("Failed to copy About text to clipboard: {err:#}"); } }); } @@ -49,32 +49,7 @@ where } fn copy_text_via_command(text: &str) -> Result<()> { - use std::io::Write; - - let mut child = std::process::Command::new("wl-copy") - .arg("--type") - .arg("text/plain") - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::piped()) - .spawn() - .context("Failed to spawn wl-copy")?; - - if let Some(mut stdin) = child.stdin.take() { - stdin - .write_all(text.as_bytes()) - .context("Failed to write to wl-copy stdin")?; - } - - let output = child - .wait_with_output() - .context("Failed to wait for wl-copy")?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(anyhow::anyhow!("wl-copy failed: {}", stderr.trim())); - } - - Ok(()) + crate::clipboard_text::copy_text_via_command(text).map_err(anyhow::Error::msg) } #[cfg(test)] @@ -83,6 +58,49 @@ mod tests { use super::*; + #[test] + fn open_url_uses_the_broker_ready_desktop_open_argv() { + let mut observed = None; + + open_url_with("https://wayscriber.com/report#d=abc", |invocation| { + observed = Some(( + invocation.program().to_owned(), + invocation.arguments().to_vec(), + )); + Ok(()) + }) + .unwrap(); + + let (program, arguments) = observed.expect("trusted URL reaches the spawn adapter"); + assert!(!matches!(program.to_str(), Some("sh" | "bash" | "cmd"))); + assert_eq!( + arguments, + [std::ffi::OsString::from( + "https://wayscriber.com/report#d=abc" + )] + ); + } + + #[test] + fn open_url_refuses_untrusted_hosts_before_spawning() { + let spawn_calls = AtomicUsize::new(0); + + for url in [ + "http://wayscriber.com/report", + "https://wayscriber.com.example/report", + "https://www.wayscriber.com/report", + "https://example.com/report", + ] { + let result = open_url_with(url, |_| { + spawn_calls.fetch_add(1, Ordering::SeqCst); + Ok(()) + }); + assert!(result.is_err(), "unexpectedly accepted {url}"); + } + + assert_eq!(spawn_calls.load(Ordering::SeqCst), 0); + } + #[test] fn copy_text_with_command_short_circuits_for_empty_text() { let command_calls = AtomicUsize::new(0); diff --git a/src/about_window/state.rs b/src/about_window/state.rs index c004348fa..647796d5f 100644 --- a/src/about_window/state.rs +++ b/src/about_window/state.rs @@ -1,6 +1,6 @@ //! About-window state: focus, hover, and the actions the handlers trigger. -use log::debug; +use log::{debug, warn}; use smithay_client_toolkit::seat::pointer::CursorIcon; use wayland_client::Connection; @@ -12,7 +12,9 @@ use super::{AboutWindowState, clipboard, icon, surface_size}; /// How the footer acknowledges an action that has no visible result of its own. const COPIED_NOTICE: &str = "Copied to clipboard"; const OPENED_NOTICE: &str = "Opened in your browser"; +const OPEN_FAILED_NOTICE: &str = "Could not open your browser — see logs"; const REPORTED_NOTICE: &str = "Diagnostics copied — paste them if the form asks"; +const REPORT_OPEN_FAILED_NOTICE: &str = "Diagnostics copied — browser open failed"; impl AboutWindowState { #[allow(clippy::too_many_arguments)] @@ -124,10 +126,13 @@ impl AboutWindowState { fn perform(&mut self, action: AboutAction) { match action { - AboutAction::OpenUrl(url) => { - clipboard::open_url(&url); - self.set_notice(OPENED_NOTICE); - } + AboutAction::OpenUrl(url) => match clipboard::open_url(&url) { + Ok(()) => self.set_notice(OPENED_NOTICE), + Err(err) => { + warn!("About dialog refused or failed to open URL {url:?}: {err:#}"); + self.set_notice(OPEN_FAILED_NOTICE); + } + }, AboutAction::CopyText(text) => { clipboard::copy_text_to_clipboard(&text); self.set_notice(COPIED_NOTICE); @@ -136,9 +141,17 @@ impl AboutWindowState { // fragment, but a browser that never launches, or a form that drops // the prefill, still leaves them one paste away. AboutAction::ReportBug { url, diagnostics } => { + // Start the non-blocking desktop-open exchange before the + // clipboard worker can take the broker transport. + let opened = clipboard::open_url(&url); clipboard::copy_text_to_clipboard(&diagnostics); - clipboard::open_url(&url); - self.set_notice(REPORTED_NOTICE); + match opened { + Ok(()) => self.set_notice(REPORTED_NOTICE), + Err(err) => { + warn!("About dialog failed to open report URL {url:?}: {err:#}"); + self.set_notice(REPORT_OPEN_FAILED_NOTICE); + } + } } AboutAction::CheckForUpdates => self.begin_update_check(), AboutAction::Close => self.should_exit = true, diff --git a/src/app/mod.rs b/src/app/mod.rs index 3282ede06..1d6ca0076 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -165,9 +165,7 @@ pub fn run(cli: Cli) -> anyhow::Result<()> { // Runtime and update-checking modes create their process broker before // acquiring locks or starting threads. The guard spans the complete run. - let update_mode_needs_broker = - !crate::update_check::compiled_out() && (cli.about || cli.check_update); - let _process_broker = (cli.daemon || cli.active || cli.freeze || update_mode_needs_broker) + let _process_broker = needs_process_broker(&cli) .then(crate::process_broker::start_for_runtime) .transpose()?; crate::daemon::protocol_v2::start_daemon_watchdog_from_environment()?; @@ -291,10 +289,36 @@ pub fn run(cli: Cli) -> anyhow::Result<()> { Ok(()) } +fn needs_process_broker(cli: &Cli) -> bool { + cli.daemon + || cli.active + || cli.freeze + // About's URL and clipboard helpers need the broker even when network + // update checks were compiled out of this build. + || cli.about + || (cli.check_update && !crate::update_check::compiled_out()) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn about_always_starts_the_process_broker() { + assert!(needs_process_broker(&Cli { + about: true, + ..Cli::default() + })); + } + + #[test] + fn print_only_mode_does_not_start_the_process_broker() { + assert!(!needs_process_broker(&Cli { + runtime_capabilities: true, + ..Cli::default() + })); + } + #[test] fn daemon_request_session_file_anchors_relative_paths_to_caller_directory() { let anchored = anchor_session_file_for_daemon_request( diff --git a/src/clipboard_text.rs b/src/clipboard_text.rs index 28c7a74f0..ed8643ae5 100644 --- a/src/clipboard_text.rs +++ b/src/clipboard_text.rs @@ -18,7 +18,7 @@ pub(crate) fn copy_text_via_command(text: &str) -> Result<(), String> { broker.publish( crate::process_broker::HelperKind::WlCopy, OsStr::new("wl-copy"), - [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8")], + clipboard_text_copy_args(), text.as_bytes().to_vec(), Duration::from_secs(5), ) @@ -38,6 +38,10 @@ pub(crate) fn copy_text_via_command(text: &str) -> Result<(), String> { Ok(()) } +fn clipboard_text_copy_args() -> [&'static OsStr; 2] { + [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8")] +} + pub(crate) fn read_clipboard_text_via_command() -> Result { let output = crate::process_broker::current() .and_then(|broker| { @@ -96,4 +100,12 @@ mod tests { ] ); } + + #[test] + fn clipboard_text_copy_publishes_one_explicit_utf8_mime() { + assert_eq!( + clipboard_text_copy_args(), + [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8"),] + ); + } } diff --git a/src/daemon/setup.rs b/src/daemon/setup.rs index 1271488ef..fc6c697ef 100644 --- a/src/daemon/setup.rs +++ b/src/daemon/setup.rs @@ -1,9 +1,10 @@ use crate::durable_io::{AtomicWriteOptions, OverwriteMode, PermissionPolicy, SymlinkPolicy}; use anyhow::{Context, Result, bail}; +use std::ffi::OsStr; use std::fs; use std::io::ErrorKind; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::time::Duration; use crate::systemd_user_service::{ USER_SERVICE_NAME, render_user_service_unit, user_service_unit_path, @@ -65,13 +66,24 @@ fn write_if_changed(path: &Path, content: &str) -> Result<()> { } fn run_systemctl_user(args: &[&str]) -> Result<()> { - let output = Command::new("systemctl") - .arg("--user") - .args(args) - .output() + let arguments = systemctl_user_arguments(args); + let output = crate::process_broker::current() + .and_then(|broker| { + broker.run( + crate::process_broker::HelperKind::Systemctl, + OsStr::new("systemctl"), + &arguments, + Vec::new(), + Duration::from_secs(30), + 256 * 1024, + ) + }) .with_context(|| format!("failed to execute systemctl --user {}", args.join(" ")))?; - if output.status.success() { + if output.timed_out { + bail!("systemctl --user {} timed out", args.join(" ")); + } + if output.status == 0 { return Ok(()); } @@ -87,6 +99,12 @@ fn run_systemctl_user(args: &[&str]) -> Result<()> { ); } +fn systemctl_user_arguments<'a>(args: &'a [&'a str]) -> Vec<&'a OsStr> { + std::iter::once(OsStr::new("--user")) + .chain(args.iter().map(|argument| OsStr::new(argument))) + .collect() +} + fn systemctl_error_detail(stdout: &str, stderr: &str) -> String { match (stdout.is_empty(), stderr.is_empty()) { (true, true) => "no output from systemctl".to_string(), @@ -95,3 +113,34 @@ fn systemctl_error_detail(stdout: &str, stderr: &str) -> String { (false, false) => format!("{stderr} | {stdout}"), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn background_setup_systemctl_argv_is_explicit_and_user_scoped() { + assert_eq!( + systemctl_user_arguments(&["daemon-reload"]), + [OsStr::new("--user"), OsStr::new("daemon-reload")] + ); + assert_eq!( + systemctl_user_arguments(&["enable", "--now", USER_SERVICE_NAME]), + [ + OsStr::new("--user"), + OsStr::new("enable"), + OsStr::new("--now"), + OsStr::new("wayscriber.service"), + ] + ); + } + + #[test] + fn systemctl_error_output_prefers_stderr_without_losing_stdout() { + assert_eq!( + systemctl_error_detail("stdout detail", "stderr detail"), + "stderr detail | stdout detail" + ); + assert_eq!(systemctl_error_detail("", ""), "no output from systemctl"); + } +} diff --git a/src/daemon/tray/helpers.rs b/src/daemon/tray/helpers.rs index 6aae46448..0db1020c6 100644 --- a/src/daemon/tray/helpers.rs +++ b/src/daemon/tray/helpers.rs @@ -39,24 +39,6 @@ fn spawn_detached( } #[cfg(feature = "tray")] -fn opener_arguments(path: &std::path::Path) -> (OsString, Vec) { - if cfg!(target_os = "macos") { - ("open".into(), vec![path.as_os_str().into()]) - } else if cfg!(target_os = "windows") { - ( - "cmd".into(), - vec![ - "/C".into(), - "start".into(), - "".into(), - path.as_os_str().into(), - ], - ) - } else { - ("xdg-open".into(), vec![path.as_os_str().into()]) - } -} - #[cfg(feature = "tray")] impl WayscriberTray { /// Open the configurator, optionally at the screen for the menu item used. @@ -142,10 +124,17 @@ impl WayscriberTray { /// Open the update instructions the watcher recorded. The URL comes from /// `update_check`, which only ever hands out validated wayscriber.com links. pub(super) fn open_update_instructions(&self, url: &str) { + let invocation = match crate::desktop_open::trusted_wayscriber_url(url) { + Ok(invocation) => invocation, + Err(err) => { + warn!("Refused to open update instructions {url:?}: {err:#}"); + return; + } + }; match spawn_detached( crate::process_broker::HelperKind::DesktopOpen, - OsStr::new("xdg-open"), - &[OsString::from(url)], + invocation.program(), + invocation.arguments(), ) { Ok(child) => info!("Opened update instructions {url} (pid {})", child.id()), Err(err) => warn!("Failed to open update instructions {url}: {err}"), @@ -201,10 +190,11 @@ impl WayscriberTray { return; } + let invocation = crate::desktop_open::path(&dir); match spawn_detached( crate::process_broker::HelperKind::DesktopOpen, - OsStr::new("xdg-open"), - &[dir.as_os_str().into()], + invocation.program(), + invocation.arguments(), ) { Ok(child) => info!("Opened log directory via xdg-open (pid {})", child.id()), Err(err) => warn!("Failed to open log directory {}: {}", dir.display(), err), @@ -220,11 +210,11 @@ impl WayscriberTray { } }; - let (opener, arguments) = opener_arguments(&path); + let invocation = crate::desktop_open::path(&path); match spawn_detached( crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, + invocation.program(), + invocation.arguments(), ) { Ok(child) => { info!( diff --git a/src/desktop_open.rs b/src/desktop_open.rs new file mode 100644 index 000000000..1d30a3c30 --- /dev/null +++ b/src/desktop_open.rs @@ -0,0 +1,109 @@ +//! Explicit desktop-opener invocations shared by runtime callers. +//! +//! The process broker authorizes the executable and cheap argument shape. This +//! module owns caller policy: paths stay paths, and outbound URLs must use the +//! trusted Wayscriber HTTPS host rule before they reach the broker. + +use std::ffi::{OsStr, OsString}; +use std::path::Path; + +use anyhow::{Result, bail}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DesktopOpenInvocation { + program: OsString, + arguments: Vec, +} + +impl DesktopOpenInvocation { + pub(crate) fn program(&self) -> &OsStr { + &self.program + } + + pub(crate) fn arguments(&self) -> &[OsString] { + &self.arguments + } +} + +/// Open a local path with the platform's desktop integration. +pub(crate) fn path(path: &Path) -> DesktopOpenInvocation { + invocation(path.as_os_str()) +} + +/// Open an HTTPS URL on the update check's exact Wayscriber host allowlist. +pub(crate) fn trusted_wayscriber_url(url: &str) -> Result { + if !crate::update_check::is_trusted_url(url) { + bail!("refusing to open an untrusted Wayscriber URL: {url:?}"); + } + Ok(invocation(OsStr::new(url))) +} + +/// About content is stricter than update metadata: every compiled-in link uses +/// the primary `https://wayscriber.com` origin. +pub(crate) fn about_url(url: &str) -> Result { + let Some(tail) = url.strip_prefix("https://wayscriber.com") else { + bail!("refusing to open an untrusted About URL: {url:?}"); + }; + if !(tail.is_empty() || tail.starts_with(['/', '?', '#'])) { + bail!("refusing to open an untrusted About URL: {url:?}"); + } + Ok(invocation(OsStr::new(url))) +} + +fn invocation(target: &OsStr) -> DesktopOpenInvocation { + // `cmd /C start` is intentionally absent: desktop opening must never route + // through a shell. Wayscriber is a Wayland application, while `open` + // preserves the existing non-shell macOS build path. + let program = if cfg!(target_os = "macos") { + "open" + } else { + "xdg-open" + }; + DesktopOpenInvocation { + program: program.into(), + arguments: vec![target.into()], + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn path_invocation_is_one_explicit_argument_without_a_shell() { + let invocation = path(Path::new("/tmp/Wayscriber Captures")); + + assert!(!matches!( + invocation.program().to_str(), + Some("sh" | "bash" | "cmd") + )); + assert_eq!( + invocation.arguments(), + [OsString::from("/tmp/Wayscriber Captures")] + ); + } + + #[test] + fn about_urls_require_https_and_an_exact_wayscriber_host() { + let invocation = about_url("https://wayscriber.com/report#d=abc").unwrap(); + assert_eq!( + invocation.arguments(), + [OsString::from("https://wayscriber.com/report#d=abc")] + ); + + for untrusted in [ + "http://wayscriber.com/report", + "https://wayscriber.com.example/report", + "https://www.wayscriber.com/report", + "https://example.com/report", + "file:///etc/passwd", + ] { + assert!( + about_url(untrusted).is_err(), + "unexpectedly accepted {untrusted}" + ); + } + + assert!(trusted_wayscriber_url("https://www.wayscriber.com/docs/").is_ok()); + } +} diff --git a/src/input/state/core/utility/launcher.rs b/src/input/state/core/utility/launcher.rs index 3f081ed7f..eb42a368f 100644 --- a/src/input/state/core/utility/launcher.rs +++ b/src/input/state/core/utility/launcher.rs @@ -42,24 +42,6 @@ fn launch_failure_message(error: &anyhow::Error, failed: &'static str) -> &'stat } } -fn opener_arguments(path: &std::path::Path) -> (OsString, Vec) { - if cfg!(target_os = "macos") { - ("open".into(), vec![path.as_os_str().into()]) - } else if cfg!(target_os = "windows") { - ( - "cmd".into(), - vec![ - "/C".into(), - "start".into(), - "".into(), - path.as_os_str().into(), - ], - ) - } else { - ("xdg-open".into(), vec![path.as_os_str().into()]) - } -} - impl InputState { /// Open the About dialog, closing the overlay first. /// @@ -175,11 +157,11 @@ impl InputState { return; }; - let (opener, arguments) = opener_arguments(&folder); + let invocation = crate::desktop_open::path(&folder); match spawn_detached( crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, + invocation.program(), + invocation.arguments(), ) { Ok(child) => { log::info!( @@ -222,11 +204,11 @@ impl InputState { } }; - let (opener, arguments) = opener_arguments(&path); + let invocation = crate::desktop_open::path(&path); match spawn_detached( crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, + invocation.program(), + invocation.arguments(), ) { Ok(child) => { log::info!( diff --git a/src/lib.rs b/src/lib.rs index a76a98efd..871b3f726 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ pub(crate) mod clipboard_text; pub mod config; pub mod configurator_destination; mod daemon; +mod desktop_open; pub mod domain; pub mod draw; pub mod durable_io; diff --git a/src/process_broker/manifest.rs b/src/process_broker/manifest.rs index c86607be2..8040a212a 100644 --- a/src/process_broker/manifest.rs +++ b/src/process_broker/manifest.rs @@ -63,12 +63,13 @@ pub(super) fn validate( HelperKind::SessionZenity => basename == "zenity", HelperKind::SessionKdialog => basename == "kdialog", HelperKind::Gsettings => basename == "gsettings", + HelperKind::Systemctl => basename == "systemctl", HelperKind::Configurator => std::env::var_os(crate::env_vars::CONFIGURATOR_ENV) .map_or_else( || basename.contains("configurator"), |configured| configured == program, ), - HelperKind::DesktopOpen => matches!(basename.as_str(), "xdg-open" | "open" | "cmd"), + HelperKind::DesktopOpen => matches!(basename.as_str(), "xdg-open" | "open"), HelperKind::UpdateFetcher => matches!(basename.as_str(), "curl" | "wget"), #[cfg(test)] HelperKind::TestSleep => basename == "sleep", @@ -80,6 +81,7 @@ pub(super) fn validate( if !allowed { bail!("program {basename:?} is not allowed for helper kind {kind:?}"); } + validate_arguments(kind, &basename, arguments)?; for (name, _) in environment { let name = std::str::from_utf8(&name.0)?; if !matches!( @@ -97,6 +99,53 @@ pub(super) fn validate( Ok(()) } +/// Cheap exec-gate checks for helpers whose safety relies on one indispensable +/// argument. Complete argv content remains the caller's policy. +fn validate_arguments(kind: HelperKind, basename: &str, arguments: &[OsWire]) -> Result<()> { + match kind { + HelperKind::UpdateFetcher => { + let required_first = if basename == "curl" { + b"--disable".as_slice() + } else { + b"--no-config".as_slice() + }; + if arguments.first().map(|argument| argument.0.as_slice()) != Some(required_first) { + bail!("update fetcher must disable user configuration in argument one"); + } + } + HelperKind::DesktopOpen => { + let [target] = arguments else { + bail!("desktop opener requires exactly one target argument"); + }; + if target.0.starts_with(b"-") { + bail!("desktop opener target must not be an option"); + } + if let Ok(target) = std::str::from_utf8(&target.0) + && looks_like_uri(target) + && !crate::update_check::is_trusted_url(target) + { + bail!("desktop opener URL is not a trusted Wayscriber HTTPS URL"); + } + } + HelperKind::Systemctl => { + if arguments.first().map(|argument| argument.0.as_slice()) != Some(b"--user") { + bail!("systemctl helper is restricted to the user service manager"); + } + } + _ => {} + } + Ok(()) +} + +fn looks_like_uri(value: &str) -> bool { + let Some((scheme, _)) = value.split_once(':') else { + return false; + }; + let mut bytes = scheme.bytes(); + bytes.next().is_some_and(|byte| byte.is_ascii_alphabetic()) + && bytes.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'+' | b'-' | b'.')) +} + pub(super) fn command( program: OsWire, arguments: Vec, diff --git a/src/process_broker/tests.rs b/src/process_broker/tests.rs index a7b84256e..3e5b59755 100644 --- a/src/process_broker/tests.rs +++ b/src/process_broker/tests.rs @@ -5,6 +5,13 @@ use std::time::{Duration, Instant}; use super::*; +fn wire_arguments(arguments: &[&str]) -> Vec { + arguments + .iter() + .map(|argument| super::wire::OsWire::from_os(OsStr::new(argument)).unwrap()) + .collect() +} + fn release_test_provider( release_path: &std::path::Path, proof_path: &std::path::Path, @@ -72,14 +79,139 @@ fn configurator_manifest_preserves_arbitrary_explicit_override_name() { #[test] fn update_fetcher_manifest_allows_only_curl_and_wget() { - for program in ["/usr/bin/curl", "/usr/bin/wget"] { + for (program, arguments) in [ + ("/usr/bin/curl", &["--disable"][..]), + ("/usr/bin/wget", &["--no-config"][..]), + ] { let program = super::wire::OsWire::from_os(OsStr::new(program)).unwrap(); - super::manifest::validate(HelperKind::UpdateFetcher, &program, &[], &[], &[]).unwrap(); + super::manifest::validate( + HelperKind::UpdateFetcher, + &program, + &wire_arguments(arguments), + &[], + &[], + ) + .unwrap(); } let unrelated = super::wire::OsWire::from_os(OsStr::new("/usr/bin/sh")).unwrap(); assert!( - super::manifest::validate(HelperKind::UpdateFetcher, &unrelated, &[], &[], &[]).is_err() + super::manifest::validate( + HelperKind::UpdateFetcher, + &unrelated, + &wire_arguments(&["--disable"]), + &[], + &[], + ) + .is_err() + ); + + for (program, arguments) in [ + ("curl", &[][..]), + ("curl", &["--silent", "--disable"][..]), + ("wget", &[][..]), + ("wget", &["--quiet", "--no-config"][..]), + ] { + let program = super::wire::OsWire::from_os(OsStr::new(program)).unwrap(); + assert!( + super::manifest::validate( + HelperKind::UpdateFetcher, + &program, + &wire_arguments(arguments), + &[], + &[], + ) + .is_err(), + "{program:?} accepted unsafe arguments {arguments:?}" + ); + } +} + +#[test] +fn desktop_open_manifest_accepts_one_path_or_trusted_url_without_a_shell() { + for target in [ + "/tmp/Wayscriber Captures", + "https://wayscriber.com/report#d=abc", + "https://www.wayscriber.com/docs/", + ] { + let program = super::wire::OsWire::from_os(OsStr::new("xdg-open")).unwrap(); + super::manifest::validate( + HelperKind::DesktopOpen, + &program, + &wire_arguments(&[target]), + &[], + &[], + ) + .unwrap(); + } + + for target in [ + "http://wayscriber.com/report", + "https://wayscriber.com.example/report", + "https://example.com/", + "file:///etc/passwd", + "--help", + ] { + let program = super::wire::OsWire::from_os(OsStr::new("xdg-open")).unwrap(); + assert!( + super::manifest::validate( + HelperKind::DesktopOpen, + &program, + &wire_arguments(&[target]), + &[], + &[], + ) + .is_err(), + "desktop-open accepted {target:?}" + ); + } + + let opener = super::wire::OsWire::from_os(OsStr::new("xdg-open")).unwrap(); + assert!(super::manifest::validate(HelperKind::DesktopOpen, &opener, &[], &[], &[]).is_err()); + assert!( + super::manifest::validate( + HelperKind::DesktopOpen, + &opener, + &wire_arguments(&["/tmp/one", "/tmp/two"]), + &[], + &[], + ) + .is_err() + ); + + let shell = super::wire::OsWire::from_os(OsStr::new("cmd")).unwrap(); + assert!( + super::manifest::validate( + HelperKind::DesktopOpen, + &shell, + &wire_arguments(&["/tmp/file"]), + &[], + &[], + ) + .is_err() + ); +} + +#[test] +fn systemctl_manifest_requires_the_user_service_manager() { + let program = super::wire::OsWire::from_os(OsStr::new("systemctl")).unwrap(); + super::manifest::validate( + HelperKind::Systemctl, + &program, + &wire_arguments(&["--user", "daemon-reload"]), + &[], + &[], + ) + .unwrap(); + assert!( + super::manifest::validate( + HelperKind::Systemctl, + &program, + &wire_arguments(&["daemon-reload"]), + &[], + &[], + ) + .is_err() ); } diff --git a/src/process_broker/wire.rs b/src/process_broker/wire.rs index 88a655f6d..ac5827606 100644 --- a/src/process_broker/wire.rs +++ b/src/process_broker/wire.rs @@ -42,6 +42,7 @@ pub(crate) enum HelperKind { SessionZenity, SessionKdialog, Gsettings, + Systemctl, Configurator, About, DesktopOpen, diff --git a/src/update_check/mod.rs b/src/update_check/mod.rs index 23cd981dd..33569ed92 100644 --- a/src/update_check/mod.rs +++ b/src/update_check/mod.rs @@ -21,6 +21,7 @@ use std::time::Duration; use log::debug; +pub(crate) use manifest::is_trusted_url; pub use manifest::{DEFAULT_NOTES_URL, DEFAULT_UPDATE_URL, MANIFEST_URL, install_source}; use crate::env_vars::DISABLE_UPDATE_CHECK_ENV; From 9b4daa61e01c191c21fda8dd4eb1d447ee7b9cd3 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:02:18 +0200 Subject: [PATCH 02/10] test: lock helper argv and temp cleanup --- src/capture/sources/hyprland.rs | 39 ++++++++-- src/ocr/tesseract.rs | 121 ++++++++++++++++++++++---------- 2 files changed, 117 insertions(+), 43 deletions(-) diff --git a/src/capture/sources/hyprland.rs b/src/capture/sources/hyprland.rs index a18e5b337..f8af2b4da 100644 --- a/src/capture/sources/hyprland.rs +++ b/src/capture/sources/hyprland.rs @@ -8,6 +8,14 @@ use std::time::Duration; // Large, noisy multi-monitor PNGs can exceed the former 16 MiB transport cap. // Keep capture bounded while allowing several uncompressed 8K-sized frames. const CAPTURE_OUTPUT_CAP: usize = 256 * 1024 * 1024; +const GRIM_FULL_SCREEN_ARGUMENTS: [&str; 1] = ["-"]; +const HYPRCTL_ACTIVE_WINDOW_ARGUMENTS: [&str; 2] = ["activewindow", "-j"]; +const HYPRCTL_MONITORS_ARGUMENTS: [&str; 2] = ["monitors", "-j"]; +const SLURP_SELECTION_ARGUMENTS: [&str; 2] = ["-f", "%x,%y %wx%h"]; + +fn grim_geometry_arguments(geometry: &str) -> [&str; 3] { + ["-g", geometry, "-"] +} fn run_helper( kind: HelperKind, @@ -37,7 +45,7 @@ pub async fn capture_full_screen_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Grim, "grim", - &["-"], + &GRIM_FULL_SCREEN_ARGUMENTS, Duration::from_secs(30), CAPTURE_OUTPUT_CAP, )?; @@ -73,7 +81,7 @@ pub async fn capture_active_window_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Hyprctl, "hyprctl", - &["activewindow", "-j"], + &HYPRCTL_ACTIVE_WINDOW_ARGUMENTS, Duration::from_secs(5), 2 * 1024 * 1024, )?; @@ -145,10 +153,11 @@ pub async fn capture_active_window_hyprland() -> Result, CaptureError> { ); log::debug!("Capturing active window via grim: {}", geometry); + let arguments = grim_geometry_arguments(&geometry); let grim_output = run_helper( HelperKind::Grim, "grim", - &["-g", &geometry, "-"], + &arguments, Duration::from_secs(30), CAPTURE_OUTPUT_CAP, )?; @@ -180,7 +189,7 @@ pub async fn capture_selection_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Slurp, "slurp", - &["-f", "%x,%y %wx%h"], + &SLURP_SELECTION_ARGUMENTS, Duration::from_secs(120), 4096, )?; @@ -208,10 +217,11 @@ pub async fn capture_selection_hyprland() -> Result, CaptureError> { } log::debug!("Capturing region via grim: {}", geometry); + let arguments = grim_geometry_arguments(geometry); let grim_output = run_helper( HelperKind::Grim, "grim", - &["-g", geometry, "-"], + &arguments, Duration::from_secs(30), CAPTURE_OUTPUT_CAP, )?; @@ -251,7 +261,7 @@ fn hyprland_monitor_scale( let output = run_helper( HelperKind::Hyprctl, "hyprctl", - &["monitors", "-j"], + &HYPRCTL_MONITORS_ARGUMENTS, Duration::from_secs(5), 2 * 1024 * 1024, )?; @@ -301,3 +311,20 @@ fn hyprland_monitor_scale( Ok(None) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn capture_helper_argv_contracts_are_explicit() { + assert_eq!(GRIM_FULL_SCREEN_ARGUMENTS, ["-"]); + assert_eq!(HYPRCTL_ACTIVE_WINDOW_ARGUMENTS, ["activewindow", "-j"]); + assert_eq!(HYPRCTL_MONITORS_ARGUMENTS, ["monitors", "-j"]); + assert_eq!(SLURP_SELECTION_ARGUMENTS, ["-f", "%x,%y %wx%h"]); + assert_eq!( + grim_geometry_arguments("12,34 800x600"), + ["-g", "12,34 800x600", "-"] + ); + } +} diff --git a/src/ocr/tesseract.rs b/src/ocr/tesseract.rs index d5ce3f7ce..43aa6c7b5 100644 --- a/src/ocr/tesseract.rs +++ b/src/ocr/tesseract.rs @@ -29,35 +29,51 @@ impl TextRecognizer for TesseractRecognizer { return Err(OcrFailure::EngineMissing); } - // A securely created temporary file with automatic deletion on every - // path. A file rather than broker stdin because the broker's input cap - // is 16 MiB and a lossless desktop crop can exceed it. - let mut input = tempfile::Builder::new() - .prefix("wayscriber-ocr-") - .suffix(".png") - .tempfile() - .map_err(|err| { - log::warn!("OCR temporary file creation failed: {err}"); - OcrFailure::TemporaryFileFailed - })?; - input - .write_all(png) - .and_then(|()| input.as_file_mut().sync_all()) - .map_err(|err| { - log::warn!("OCR temporary file write failed: {err}"); - OcrFailure::TemporaryFileFailed - })?; - - let output = run_tesseract(input.path(), languages); - // Explicit close so the deletion error is observable rather than - // swallowed by `Drop`; either way the file is gone before returning. - if let Err(err) = input.close() { - log::warn!("OCR temporary file cleanup failed: {err}"); - } - output + with_temporary_png(png, |input| run_tesseract(input, languages)) } } +/// Run one OCR operation with a securely created PNG that cannot outlive the +/// stack frame, including while unwinding from a panic. +fn with_temporary_png( + png: &[u8], + operation: impl FnOnce(&Path) -> Result, +) -> Result { + with_temporary_png_in(png, &std::env::temp_dir(), operation) +} + +fn with_temporary_png_in( + png: &[u8], + directory: &Path, + operation: impl FnOnce(&Path) -> Result, +) -> Result { + // A file rather than broker stdin because the broker's input cap is 16 MiB + // and a lossless desktop crop can exceed it. NamedTempFile's Drop removes + // the path during unwinding; explicit close makes ordinary cleanup errors + // observable before the operation returns. + let mut input = tempfile::Builder::new() + .prefix("wayscriber-ocr-") + .suffix(".png") + .tempfile_in(directory) + .map_err(|err| { + log::warn!("OCR temporary file creation failed: {err}"); + OcrFailure::TemporaryFileFailed + })?; + input + .write_all(png) + .and_then(|()| input.as_file_mut().sync_all()) + .map_err(|err| { + log::warn!("OCR temporary file write failed: {err}"); + OcrFailure::TemporaryFileFailed + })?; + + let output = operation(input.path()); + if let Err(err) = input.close() { + log::warn!("OCR temporary file cleanup failed: {err}"); + } + output +} + fn run_tesseract(input: &Path, languages: &OcrLanguages) -> Result { let output = crate::process_broker::current() .and_then(|broker| { @@ -295,19 +311,50 @@ mod tests { } #[test] - fn a_temporary_input_file_is_removed_on_every_path() { - // The recognizer owns the file through `tempfile`; the guarantee under - // test is that no wayscriber-ocr file survives a run that fails before, - // during, or after the engine call. Without a broker the run fails at - // the invocation step, which is the hardest path to clean up. - let before = temporary_ocr_files(); - let outcome = TesseractRecognizer.recognize(b"not a png", &languages()); - assert!(outcome.is_err()); - assert_eq!(temporary_ocr_files(), before); + fn temporary_input_is_removed_after_success() { + let directory = crate::test_temp::tempdir().unwrap(); + + let result = with_temporary_png_in(b"png", directory.path(), |path| { + assert!(path.exists()); + assert_eq!(std::fs::read(path).unwrap(), b"png"); + Ok("recognized") + }); + + assert_eq!(result.unwrap(), "recognized"); + assert!(temporary_ocr_files(directory.path()).is_empty()); + } + + #[test] + fn temporary_input_is_removed_after_operation_failure() { + let directory = crate::test_temp::tempdir().unwrap(); + + let result = with_temporary_png_in(b"png", directory.path(), |path| { + assert!(path.exists()); + Err::<(), _>(OcrFailure::EngineFailed) + }); + + assert!(matches!(result, Err(OcrFailure::EngineFailed))); + assert!(temporary_ocr_files(directory.path()).is_empty()); + } + + #[test] + fn temporary_input_is_removed_while_unwinding_from_a_panic() { + let directory = crate::test_temp::tempdir().unwrap(); + + let panic = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _: Result<(), OcrFailure> = + with_temporary_png_in(b"png", directory.path(), |path| { + assert!(path.exists()); + panic!("simulated OCR adapter panic"); + }); + })); + + assert!(panic.is_err()); + assert!(temporary_ocr_files(directory.path()).is_empty()); } - fn temporary_ocr_files() -> Vec { - let Ok(entries) = std::fs::read_dir(std::env::temp_dir()) else { + fn temporary_ocr_files(directory: &Path) -> Vec { + let Ok(entries) = std::fs::read_dir(directory) else { return Vec::new(); }; let mut paths: Vec<_> = entries From def16dc86606458d150d12a5cff78a649c858392 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:03:42 +0200 Subject: [PATCH 03/10] fix: surface gsettings helper failures --- src/capture/clipboard.rs | 2 +- src/daemon/tray/shortcut_hint_io.rs | 63 +++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/capture/clipboard.rs b/src/capture/clipboard.rs index eabf6d184..3702f8f13 100644 --- a/src/capture/clipboard.rs +++ b/src/capture/clipboard.rs @@ -31,7 +31,7 @@ where Ok(()) } -/// Copy to clipboard by shelling out to wl-copy command. +/// Copy to the clipboard through the brokered `wl-copy` helper. fn copy_via_command(image_data: &[u8]) -> Result<(), CaptureError> { let output = crate::process_broker::current() .and_then(|broker| { diff --git a/src/daemon/tray/shortcut_hint_io.rs b/src/daemon/tray/shortcut_hint_io.rs index dc6b7cf37..c7b35b8ea 100644 --- a/src/daemon/tray/shortcut_hint_io.rs +++ b/src/daemon/tray/shortcut_hint_io.rs @@ -54,20 +54,57 @@ fn read_gnome_shortcut_outputs() -> Option<(String, String)> { #[cfg(feature = "tray")] fn read_gsettings_value(schema: &str, key: &str) -> Option { - let output = crate::process_broker::current() - .and_then(|broker| { - broker.run( - crate::process_broker::HelperKind::Gsettings, - OsStr::new("gsettings"), - [OsStr::new("get"), OsStr::new(schema), OsStr::new(key)], - Vec::new(), - Duration::from_secs(3), - 64 * 1024, - ) - }) - .ok()?; - if output.timed_out || output.status != 0 { + let arguments = gsettings_get_arguments(schema, key); + let output = match crate::process_broker::current().and_then(|broker| { + broker.run( + crate::process_broker::HelperKind::Gsettings, + OsStr::new("gsettings"), + arguments, + Vec::new(), + Duration::from_secs(3), + 64 * 1024, + ) + }) { + Ok(output) => output, + Err(err) => { + log::warn!( + "Failed to query the GNOME shortcut hint through the process broker: {err:#}" + ); + return None; + } + }; + if output.timed_out { + log::warn!("Timed out while querying the GNOME shortcut hint with gsettings"); + return None; + } + if output.status != 0 { + log::warn!( + "gsettings could not read the GNOME shortcut hint (status {})", + output.status + ); return None; } Some(String::from_utf8_lossy(&output.stdout).to_string()) } + +#[cfg(feature = "tray")] +fn gsettings_get_arguments<'a>(schema: &'a str, key: &'a str) -> [&'a OsStr; 3] { + [OsStr::new("get"), OsStr::new(schema), OsStr::new(key)] +} + +#[cfg(all(test, feature = "tray"))] +mod tests { + use super::*; + + #[test] + fn gsettings_shortcut_query_has_an_explicit_read_only_argv() { + assert_eq!( + gsettings_get_arguments("org.example.settings", "binding"), + [ + OsStr::new("get"), + OsStr::new("org.example.settings"), + OsStr::new("binding"), + ] + ); + } +} From 7977ac0fe670a22ba632f40dfa3dbbbc60df439c Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:06:21 +0200 Subject: [PATCH 04/10] fix: keep helper policies lint-clean --- src/daemon/setup.rs | 2 +- src/desktop_open.rs | 1 + src/process_broker/manifest.rs | 8 ++++---- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/daemon/setup.rs b/src/daemon/setup.rs index fc6c697ef..16a517efb 100644 --- a/src/daemon/setup.rs +++ b/src/daemon/setup.rs @@ -101,7 +101,7 @@ fn run_systemctl_user(args: &[&str]) -> Result<()> { fn systemctl_user_arguments<'a>(args: &'a [&'a str]) -> Vec<&'a OsStr> { std::iter::once(OsStr::new("--user")) - .chain(args.iter().map(|argument| OsStr::new(argument))) + .chain(args.iter().map(OsStr::new)) .collect() } diff --git a/src/desktop_open.rs b/src/desktop_open.rs index 1d30a3c30..50197424f 100644 --- a/src/desktop_open.rs +++ b/src/desktop_open.rs @@ -31,6 +31,7 @@ pub(crate) fn path(path: &Path) -> DesktopOpenInvocation { } /// Open an HTTPS URL on the update check's exact Wayscriber host allowlist. +#[cfg(any(feature = "tray", test))] pub(crate) fn trusted_wayscriber_url(url: &str) -> Result { if !crate::update_check::is_trusted_url(url) { bail!("refusing to open an untrusted Wayscriber URL: {url:?}"); diff --git a/src/process_broker/manifest.rs b/src/process_broker/manifest.rs index 8040a212a..373a35e0e 100644 --- a/src/process_broker/manifest.rs +++ b/src/process_broker/manifest.rs @@ -127,10 +127,10 @@ fn validate_arguments(kind: HelperKind, basename: &str, arguments: &[OsWire]) -> bail!("desktop opener URL is not a trusted Wayscriber HTTPS URL"); } } - HelperKind::Systemctl => { - if arguments.first().map(|argument| argument.0.as_slice()) != Some(b"--user") { - bail!("systemctl helper is restricted to the user service manager"); - } + HelperKind::Systemctl + if arguments.first().map(|argument| argument.0.as_slice()) != Some(b"--user") => + { + bail!("systemctl helper is restricted to the user service manager"); } _ => {} } From 15ea8532c57e47548a893c8e70cd34b2cebdc1f4 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:12:12 +0200 Subject: [PATCH 05/10] test: forbid restored process bypasses --- tools/check-process-sites.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/check-process-sites.py b/tools/check-process-sites.py index e3048c255..793b77e32 100755 --- a/tools/check-process-sites.py +++ b/tools/check-process-sites.py @@ -12,8 +12,6 @@ BROKER_ROOT = Path("src/process_broker") BROKER_BOOTSTRAP = BROKER_ROOT / "bootstrap.rs" DIRECT_PRODUCTION_ALLOWLIST = { - Path("src/about_window/clipboard.rs"), # standalone, descriptor-free About process - Path("src/daemon/setup.rs"), # pre-runtime systemd setup Path("configurator/src/app/session_catalog.rs"), # separate configurator process Path("configurator/src/app/daemon_setup/command.rs"), Path("configurator/src/app/daemon_setup/service.rs"), From a79d4254fd258454c4f08372e3f12a0e13159466 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:19:14 +0200 Subject: [PATCH 06/10] fix: bound desktop-open and clipboard helpers --- src/about_window/clipboard.rs | 20 +--- src/about_window/state.rs | 14 +-- .../wayland/clipboard/system/command.rs | 1 + src/capture/clipboard.rs | 1 + src/clipboard_text.rs | 1 + src/daemon/tray/helpers.rs | 31 ++---- src/desktop_open.rs | 102 +++++++++++++++++- src/input/state/core/utility/launcher.rs | 28 ++--- src/process_broker/client.rs | 2 + src/process_broker/server.rs | 4 + src/process_broker/tests.rs | 27 +++++ src/process_broker/wire.rs | 2 + 12 files changed, 164 insertions(+), 69 deletions(-) diff --git a/src/about_window/clipboard.rs b/src/about_window/clipboard.rs index 3c4951be9..b048c2786 100644 --- a/src/about_window/clipboard.rs +++ b/src/about_window/clipboard.rs @@ -3,26 +3,16 @@ use log::warn; pub(super) fn open_url(url: &str) -> Result<()> { open_url_with(url, |invocation| { - // This runs from a Wayland protocol handler. Match the overlay - // launchers by refusing a contended broker exchange instead of - // stalling input and redraw behind another helper. - crate::process_broker::current()?.try_spawn( - crate::process_broker::HelperKind::DesktopOpen, - crate::process_broker::HelperLifetime::DetachedAfterExec, - invocation.program(), - invocation.arguments(), - Vec::new(), - )?; - Ok(()) + crate::desktop_open::open_in_background(invocation.clone()) }) } fn open_url_with( url: &str, - spawn: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> Result<()>, + open: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> Result<()>, ) -> Result<()> { let invocation = crate::desktop_open::about_url(url)?; - spawn(&invocation) + open(&invocation) } pub(super) fn copy_text_to_clipboard(text: &str) { @@ -59,7 +49,7 @@ mod tests { use super::*; #[test] - fn open_url_uses_the_broker_ready_desktop_open_argv() { + fn open_url_builds_the_broker_ready_desktop_open_argv() { let mut observed = None; open_url_with("https://wayscriber.com/report#d=abc", |invocation| { @@ -71,7 +61,7 @@ mod tests { }) .unwrap(); - let (program, arguments) = observed.expect("trusted URL reaches the spawn adapter"); + let (program, arguments) = observed.expect("trusted URL reaches the open adapter"); assert!(!matches!(program.to_str(), Some("sh" | "bash" | "cmd"))); assert_eq!( arguments, diff --git a/src/about_window/state.rs b/src/about_window/state.rs index 647796d5f..634dc85b6 100644 --- a/src/about_window/state.rs +++ b/src/about_window/state.rs @@ -11,9 +11,9 @@ use super::{AboutWindowState, clipboard, icon, surface_size}; /// How the footer acknowledges an action that has no visible result of its own. const COPIED_NOTICE: &str = "Copied to clipboard"; -const OPENED_NOTICE: &str = "Opened in your browser"; +const OPENING_NOTICE: &str = "Opening in your browser"; const OPEN_FAILED_NOTICE: &str = "Could not open your browser — see logs"; -const REPORTED_NOTICE: &str = "Diagnostics copied — paste them if the form asks"; +const REPORTED_NOTICE: &str = "Diagnostics copied — opening browser"; const REPORT_OPEN_FAILED_NOTICE: &str = "Diagnostics copied — browser open failed"; impl AboutWindowState { @@ -127,9 +127,9 @@ impl AboutWindowState { fn perform(&mut self, action: AboutAction) { match action { AboutAction::OpenUrl(url) => match clipboard::open_url(&url) { - Ok(()) => self.set_notice(OPENED_NOTICE), + Ok(()) => self.set_notice(OPENING_NOTICE), Err(err) => { - warn!("About dialog refused or failed to open URL {url:?}: {err:#}"); + warn!("About dialog refused or failed to open a URL: {err:#}"); self.set_notice(OPEN_FAILED_NOTICE); } }, @@ -141,14 +141,14 @@ impl AboutWindowState { // fragment, but a browser that never launches, or a form that drops // the prefill, still leaves them one paste away. AboutAction::ReportBug { url, diagnostics } => { - // Start the non-blocking desktop-open exchange before the - // clipboard worker can take the broker transport. + // Queue the bounded desktop-open worker before the independent + // clipboard publication worker. let opened = clipboard::open_url(&url); clipboard::copy_text_to_clipboard(&diagnostics); match opened { Ok(()) => self.set_notice(REPORTED_NOTICE), Err(err) => { - warn!("About dialog failed to open report URL {url:?}: {err:#}"); + warn!("About dialog failed to open the report URL: {err:#}"); self.set_notice(REPORT_OPEN_FAILED_NOTICE); } } diff --git a/src/backend/wayland/clipboard/system/command.rs b/src/backend/wayland/clipboard/system/command.rs index 86c12a056..bf21cfaab 100644 --- a/src/backend/wayland/clipboard/system/command.rs +++ b/src/backend/wayland/clipboard/system/command.rs @@ -51,6 +51,7 @@ impl ClipboardCommandRunner for WlClipboardCommandRunner { [OsStr::new("--type"), OsStr::new(WAYSCRIBER_SELECTION_MIME)], payload.to_vec(), timeout, + 0, ) } } diff --git a/src/capture/clipboard.rs b/src/capture/clipboard.rs index 3702f8f13..0f4ab698b 100644 --- a/src/capture/clipboard.rs +++ b/src/capture/clipboard.rs @@ -41,6 +41,7 @@ fn copy_via_command(image_data: &[u8]) -> Result<(), CaptureError> { [OsStr::new("--type"), OsStr::new("image/png")], image_data.to_vec(), Duration::from_secs(10), + 0, ) }) .map_err(|e| { diff --git a/src/clipboard_text.rs b/src/clipboard_text.rs index ed8643ae5..a1abcf7ef 100644 --- a/src/clipboard_text.rs +++ b/src/clipboard_text.rs @@ -21,6 +21,7 @@ pub(crate) fn copy_text_via_command(text: &str) -> Result<(), String> { clipboard_text_copy_args(), text.as_bytes().to_vec(), Duration::from_secs(5), + 0, ) }) .map_err(|error| format!("Failed to run wl-copy: {error:#}"))?; diff --git a/src/daemon/tray/helpers.rs b/src/daemon/tray/helpers.rs index 0db1020c6..00e3a3ce4 100644 --- a/src/daemon/tray/helpers.rs +++ b/src/daemon/tray/helpers.rs @@ -38,7 +38,6 @@ fn spawn_detached( ) } -#[cfg(feature = "tray")] #[cfg(feature = "tray")] impl WayscriberTray { /// Open the configurator, optionally at the screen for the menu item used. @@ -131,12 +130,8 @@ impl WayscriberTray { return; } }; - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - invocation.program(), - invocation.arguments(), - ) { - Ok(child) => info!("Opened update instructions {url} (pid {})", child.id()), + match crate::desktop_open::open_in_background(invocation) { + Ok(()) => info!("Opening update instructions"), Err(err) => warn!("Failed to open update instructions {url}: {err}"), } } @@ -191,12 +186,8 @@ impl WayscriberTray { } let invocation = crate::desktop_open::path(&dir); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - invocation.program(), - invocation.arguments(), - ) { - Ok(child) => info!("Opened log directory via xdg-open (pid {})", child.id()), + match crate::desktop_open::open_in_background(invocation) { + Ok(()) => info!("Opening log directory via desktop integration"), Err(err) => warn!("Failed to open log directory {}: {}", dir.display(), err), } } @@ -211,17 +202,9 @@ impl WayscriberTray { }; let invocation = crate::desktop_open::path(&path); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - invocation.program(), - invocation.arguments(), - ) { - Ok(child) => { - info!( - "Opened config file at {} (pid {})", - path.display(), - child.id() - ); + match crate::desktop_open::open_in_background(invocation) { + Ok(()) => { + info!("Opening config file at {}", path.display()); true } Err(err) => { diff --git a/src/desktop_open.rs b/src/desktop_open.rs index 50197424f..6fae935ad 100644 --- a/src/desktop_open.rs +++ b/src/desktop_open.rs @@ -6,8 +6,12 @@ use std::ffi::{OsStr, OsString}; use std::path::Path; +use std::time::Duration; -use anyhow::{Result, bail}; +use anyhow::{Context, Result, bail}; + +const HELPER_TIMEOUT: Duration = Duration::from_secs(10); +const OUTPUT_CAP: usize = 16 * 1024; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct DesktopOpenInvocation { @@ -25,6 +29,60 @@ impl DesktopOpenInvocation { } } +/// Run a desktop opener without blocking the Wayland or tray callback that +/// requested it. The broker still owns the complete helper lifetime and +/// enforces the timeout/output policy inside the worker. +pub(crate) fn open_in_background(invocation: DesktopOpenInvocation) -> Result<()> { + let broker = crate::process_broker::current()?; + std::thread::Builder::new() + .name("wayscriber-desktop-open".to_string()) + .spawn(move || { + if let Err(err) = run_with(&invocation, |program, arguments, timeout, output_cap| { + broker.run( + crate::process_broker::HelperKind::DesktopOpen, + program, + arguments, + Vec::new(), + timeout, + output_cap, + ) + }) { + // Do not include the target or captured output: an About report + // URL can carry diagnostics in its fragment. + log::warn!("Desktop opener failed: {err:#}"); + } + }) + .context("failed to start desktop-open worker")?; + Ok(()) +} + +fn run_with( + invocation: &DesktopOpenInvocation, + run: impl FnOnce( + &OsStr, + &[OsString], + Duration, + usize, + ) -> Result, +) -> Result<()> { + let output = run( + invocation.program(), + invocation.arguments(), + HELPER_TIMEOUT, + OUTPUT_CAP, + )?; + if output.timed_out { + bail!("desktop opener timed out"); + } + if output.status != 0 { + bail!( + "desktop opener exited unsuccessfully with status {}", + output.status + ); + } + Ok(()) +} + /// Open a local path with the platform's desktop integration. pub(crate) fn path(path: &Path) -> DesktopOpenInvocation { invocation(path.as_os_str()) @@ -84,6 +142,48 @@ mod tests { ); } + #[test] + fn desktop_open_run_declares_argv_timeout_and_output_cap() { + let invocation = path(Path::new("/tmp/Wayscriber Captures")); + let mut observed = None; + + run_with(&invocation, |program, arguments, timeout, output_cap| { + observed = Some((program.to_owned(), arguments.to_vec(), timeout, output_cap)); + Ok(crate::process_broker::BrokerOutput { + status: 0, + stdout: Vec::new(), + stderr: Vec::new(), + timed_out: false, + stdout_limit_reached: false, + }) + }) + .unwrap(); + + let (program, arguments, timeout, output_cap) = observed.unwrap(); + assert!(!matches!(program.to_str(), Some("sh" | "bash" | "cmd"))); + assert_eq!(arguments, [OsString::from("/tmp/Wayscriber Captures")]); + assert_eq!(timeout, Duration::from_secs(10)); + assert_eq!(output_cap, 16 * 1024); + } + + #[test] + fn desktop_open_run_surfaces_timeout_and_nonzero_exit() { + let invocation = path(Path::new("/tmp/capture")); + let output = |status, timed_out| crate::process_broker::BrokerOutput { + status, + stdout: Vec::new(), + stderr: Vec::new(), + timed_out, + stdout_limit_reached: false, + }; + + let timeout = run_with(&invocation, |_, _, _, _| Ok(output(137, true))).unwrap_err(); + assert!(timeout.to_string().contains("timed out")); + + let nonzero = run_with(&invocation, |_, _, _, _| Ok(output(4, false))).unwrap_err(); + assert!(nonzero.to_string().contains("status 4")); + } + #[test] fn about_urls_require_https_and_an_exact_wayscriber_host() { let invocation = about_url("https://wayscriber.com/report#d=abc").unwrap(); diff --git a/src/input/state/core/utility/launcher.rs b/src/input/state/core/utility/launcher.rs index eb42a368f..3218bfda2 100644 --- a/src/input/state/core/utility/launcher.rs +++ b/src/input/state/core/utility/launcher.rs @@ -158,17 +158,9 @@ impl InputState { }; let invocation = crate::desktop_open::path(&folder); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - invocation.program(), - invocation.arguments(), - ) { - Ok(child) => { - log::info!( - "Opened capture folder at {} (pid {})", - folder.display(), - child.id() - ); + match crate::desktop_open::open_in_background(invocation) { + Ok(()) => { + log::info!("Opening capture folder at {}", folder.display()); self.should_exit = true; } Err(err) => { @@ -205,17 +197,9 @@ impl InputState { }; let invocation = crate::desktop_open::path(&path); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - invocation.program(), - invocation.arguments(), - ) { - Ok(child) => { - log::info!( - "Opened config file at {} (pid {})", - path.display(), - child.id() - ); + match crate::desktop_open::open_in_background(invocation) { + Ok(()) => { + log::info!("Opening config file at {}", path.display()); self.should_exit = true; true } diff --git a/src/process_broker/client.rs b/src/process_broker/client.rs index 174b8347e..48cb404c9 100644 --- a/src/process_broker/client.rs +++ b/src/process_broker/client.rs @@ -372,6 +372,7 @@ impl ProcessBroker { arguments: I, input: Vec, timeout: Duration, + output_cap: usize, ) -> Result where I: IntoIterator, @@ -396,6 +397,7 @@ impl ProcessBroker { environment: Vec::new(), input, timeout_ms: u64::try_from(timeout.as_millis()).unwrap_or(u64::MAX), + output_cap: output_cap.min(MAX_OUTPUT_BYTES), }, &request_descriptors, ExchangeWait::Queue, diff --git a/src/process_broker/server.rs b/src/process_broker/server.rs index 442345aa7..54759cdcb 100644 --- a/src/process_broker/server.rs +++ b/src/process_broker/server.rs @@ -293,6 +293,7 @@ fn handle_operation( environment, input, timeout_ms, + output_cap, } => { if !supports_retained_publication(kind) { bail!("retained publication is restricted to wl-copy"); @@ -300,6 +301,9 @@ fn handle_operation( let input = decode_blob(input, descriptors, super::manifest::input_cap(kind))?; reject_descriptors(descriptors)?; super::manifest::validate(kind, &program, &arguments, &environment, &input)?; + if output_cap != 0 { + bail!("retained publication output cap must be zero"); + } let output = publish_bounded( super::manifest::command(program, arguments, environment), input, diff --git a/src/process_broker/tests.rs b/src/process_broker/tests.rs index 3e5b59755..ca4eee70d 100644 --- a/src/process_broker/tests.rs +++ b/src/process_broker/tests.rs @@ -668,6 +668,7 @@ fn normal_broker_shutdown_releases_successful_provider_descendant() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(output.status, 0); @@ -703,6 +704,7 @@ fn shutdown_channel_peer_loss_kills_retained_provider() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(output.status, 0); @@ -760,6 +762,7 @@ fn retained_publication_replacement_disposes_the_previous_provider() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(first.status, 0); @@ -780,6 +783,7 @@ fn retained_publication_replacement_disposes_the_previous_provider() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(second.status, 0); @@ -842,6 +846,7 @@ fn failed_publication_replacement_preserves_the_current_provider() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(current.status, 0); @@ -859,6 +864,7 @@ fn failed_publication_replacement_preserves_the_current_provider() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(failed.status, 7); @@ -919,6 +925,7 @@ fn retained_publication_kills_failed_or_input_stalled_provider_groups() { ], input, Duration::from_millis(100), + 0, ); let provider_pid = std::fs::read_to_string(pid_path) .unwrap() @@ -954,11 +961,28 @@ fn retained_publication_rejects_incomplete_input_after_successful_exit() { [OsStr::new("-c"), OsStr::new("exit 0")], vec![b'x'; 1024 * 1024], Duration::from_secs(1), + 0, ); assert!(result.is_err(), "incomplete publication input was accepted"); } +#[test] +fn retained_publication_requires_an_explicit_zero_output_cap() { + let guard = start_for_runtime().unwrap(); + let result = guard.broker().publish( + HelperKind::TestShell, + OsStr::new("sh"), + [OsStr::new("-c"), OsStr::new("exit 0")], + Vec::new(), + Duration::from_secs(1), + 1, + ); + + let error = result.expect_err("nonzero publication output cap was accepted"); + assert!(format!("{error:#}").contains("output cap must be zero")); +} + #[test] fn broker_shutdown_preempts_retained_publication_stdin_writer() { let guard = start_for_runtime().unwrap(); @@ -985,6 +1009,7 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { ], Vec::new(), Duration::from_secs(2), + 0, ) .unwrap(); assert_eq!(current.status, 0); @@ -1007,6 +1032,7 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { ], vec![b'x'; 1024 * 1024], Duration::from_secs(2), + 0, ) } }); @@ -1070,6 +1096,7 @@ fn wl_copy_publication_accepts_capture_sized_input() { // observed to exceed 5 s when the full parallel suite saturates // the machine - the broker then SIGKILLs the helper (status 137). Duration::from_secs(30), + 0, ) .unwrap(); diff --git a/src/process_broker/wire.rs b/src/process_broker/wire.rs index ac5827606..4240b1425 100644 --- a/src/process_broker/wire.rs +++ b/src/process_broker/wire.rs @@ -91,6 +91,7 @@ pub(super) enum BrokerOperation { environment: Vec<(OsWire, Option)>, input: BlobWire, timeout_ms: u64, + /// Retained publication discards output, so callers must declare zero. output_cap: usize, output_mode: OutputMode, }, @@ -101,6 +102,7 @@ pub(super) enum BrokerOperation { environment: Vec<(OsWire, Option)>, input: BlobWire, timeout_ms: u64, + output_cap: usize, }, Spawn { kind: HelperKind, From 1f164640bd4effbcf55fe823617afda4cc5186a2 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:23:40 +0200 Subject: [PATCH 07/10] fix: finish desktop-open before overlay teardown --- src/desktop_open.rs | 17 ++++++ src/input/state/core/utility/launcher.rs | 68 +++++++++++++++++++++--- src/process_broker/wire.rs | 2 +- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/desktop_open.rs b/src/desktop_open.rs index 6fae935ad..8b2c18d6a 100644 --- a/src/desktop_open.rs +++ b/src/desktop_open.rs @@ -29,6 +29,23 @@ impl DesktopOpenInvocation { } } +/// Complete a bounded desktop-open operation before the caller continues. +/// Runtime owners that exit after the action use this form so broker teardown +/// cannot cancel the helper they just requested. +pub(crate) fn open(invocation: &DesktopOpenInvocation) -> Result<()> { + let broker = crate::process_broker::current()?; + run_with(invocation, |program, arguments, timeout, output_cap| { + broker.run( + crate::process_broker::HelperKind::DesktopOpen, + program, + arguments, + Vec::new(), + timeout, + output_cap, + ) + }) +} + /// Run a desktop opener without blocking the Wayland or tray callback that /// requested it. The broker still owns the complete helper lifetime and /// enforces the timeout/output policy inside the worker. diff --git a/src/input/state/core/utility/launcher.rs b/src/input/state/core/utility/launcher.rs index 3218bfda2..3a319ee69 100644 --- a/src/input/state/core/utility/launcher.rs +++ b/src/input/state/core/utility/launcher.rs @@ -42,6 +42,26 @@ fn launch_failure_message(error: &anyhow::Error, failed: &'static str) -> &'stat } } +/// Complete desktop-open before requesting runtime exit. Dropping the runtime +/// also tears down its broker, so reversing these operations would race the +/// bounded helper against broker shutdown. +fn open_before_runtime_exit( + invocation: &crate::desktop_open::DesktopOpenInvocation, + should_exit: &mut bool, +) -> anyhow::Result<()> { + open_before_runtime_exit_with(invocation, should_exit, crate::desktop_open::open) +} + +fn open_before_runtime_exit_with( + invocation: &crate::desktop_open::DesktopOpenInvocation, + should_exit: &mut bool, + open: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> anyhow::Result<()>, +) -> anyhow::Result<()> { + open(invocation)?; + *should_exit = true; + Ok(()) +} + impl InputState { /// Open the About dialog, closing the overlay first. /// @@ -158,10 +178,9 @@ impl InputState { }; let invocation = crate::desktop_open::path(&folder); - match crate::desktop_open::open_in_background(invocation) { + match open_before_runtime_exit(&invocation, &mut self.should_exit) { Ok(()) => { - log::info!("Opening capture folder at {}", folder.display()); - self.should_exit = true; + log::info!("Opened capture folder at {}", folder.display()); } Err(err) => { log::error!( @@ -197,10 +216,9 @@ impl InputState { }; let invocation = crate::desktop_open::path(&path); - match crate::desktop_open::open_in_background(invocation) { + match open_before_runtime_exit(&invocation, &mut self.should_exit) { Ok(()) => { - log::info!("Opening config file at {}", path.display()); - self.should_exit = true; + log::info!("Opened config file at {}", path.display()); true } Err(err) => { @@ -215,3 +233,41 @@ impl InputState { } } } + +#[cfg(test)] +mod tests { + use std::cell::Cell; + use std::path::Path; + + use super::*; + + #[test] + fn desktop_open_must_complete_before_runtime_exit_is_requested() { + let invocation = crate::desktop_open::path(Path::new("/tmp/capture")); + let completed = Cell::new(false); + let mut should_exit = false; + + open_before_runtime_exit_with(&invocation, &mut should_exit, |_| { + completed.set(true); + Ok(()) + }) + .unwrap(); + + assert!(completed.get()); + assert!(should_exit); + } + + #[test] + fn failed_desktop_open_keeps_the_runtime_and_broker_owner_alive() { + let invocation = crate::desktop_open::path(Path::new("/tmp/capture")); + let mut should_exit = false; + + let error = open_before_runtime_exit_with(&invocation, &mut should_exit, |_| { + Err(anyhow::anyhow!("injected desktop-open failure")) + }) + .unwrap_err(); + + assert!(error.to_string().contains("injected desktop-open failure")); + assert!(!should_exit); + } +} diff --git a/src/process_broker/wire.rs b/src/process_broker/wire.rs index 4240b1425..23853c3ff 100644 --- a/src/process_broker/wire.rs +++ b/src/process_broker/wire.rs @@ -91,7 +91,6 @@ pub(super) enum BrokerOperation { environment: Vec<(OsWire, Option)>, input: BlobWire, timeout_ms: u64, - /// Retained publication discards output, so callers must declare zero. output_cap: usize, output_mode: OutputMode, }, @@ -102,6 +101,7 @@ pub(super) enum BrokerOperation { environment: Vec<(OsWire, Option)>, input: BlobWire, timeout_ms: u64, + /// Retained publication discards output, so callers must declare zero. output_cap: usize, }, Spawn { From b89785eb40f7975cf5eec25c6f4dd1d9addb54e5 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:32:06 +0200 Subject: [PATCH 08/10] fix: own desktop-open completion in Wayland runtime --- .../wayland/backend/event_loop/capture.rs | 2 + src/backend/wayland/backend/event_loop/mod.rs | 1 + src/backend/wayland/clipboard/mod.rs | 2 +- src/backend/wayland/state.rs | 5 + src/backend/wayland/state/core/init.rs | 5 + src/backend/wayland/state/desktop_open.rs | 209 ++++++++++++++++++ src/desktop_open.rs | 72 ++++-- src/input/state/core/base/types.rs | 1 + src/input/state/core/utility/launcher.rs | 108 +-------- src/input/state/core/utility/pending.rs | 14 ++ src/process_broker/mod.rs | 2 +- 11 files changed, 301 insertions(+), 120 deletions(-) create mode 100644 src/backend/wayland/state/desktop_open.rs diff --git a/src/backend/wayland/backend/event_loop/capture.rs b/src/backend/wayland/backend/event_loop/capture.rs index 1a4528d43..8460495dd 100644 --- a/src/backend/wayland/backend/event_loop/capture.rs +++ b/src/backend/wayland/backend/event_loop/capture.rs @@ -94,6 +94,7 @@ pub(super) fn handle_pending_actions( state.poll_text_paste_completion(); state.poll_ocr_completion(); state.poll_session_file_dialog_completion(qh); + state.poll_desktop_open_completion(); state.drain_clipboard_requests(); state.handle_pending_eyedropper_toggle(); state.handle_pending_ocr_request(); @@ -134,6 +135,7 @@ pub(super) fn handle_pending_actions( PendingBackendAction::BoardPdfExport(action) => { state.handle_board_pdf_export_action(action); } + PendingBackendAction::DesktopOpen(request) => state.handle_desktop_open(request), PendingBackendAction::ClearSavedToolState => { state.handle_clear_saved_tool_state_action(); } diff --git a/src/backend/wayland/backend/event_loop/mod.rs b/src/backend/wayland/backend/event_loop/mod.rs index 653f5b5a6..58fb5d5ce 100644 --- a/src/backend/wayland/backend/event_loop/mod.rs +++ b/src/backend/wayland/backend/event_loop/mod.rs @@ -211,6 +211,7 @@ pub(super) fn run_event_loop( } if state.surface.is_xdg_window() && !state.has_keyboard_focus() + && !state.desktop_open_in_progress() && state.focus_exit_suppression_expired(Instant::now()) { if state.xdg_focus_loss_exits_overlay() { diff --git a/src/backend/wayland/clipboard/mod.rs b/src/backend/wayland/clipboard/mod.rs index bac9484ec..6fcb05df6 100644 --- a/src/backend/wayland/clipboard/mod.rs +++ b/src/backend/wayland/clipboard/mod.rs @@ -17,7 +17,7 @@ mod image; mod system; pub(in crate::backend::wayland) mod transfer; pub(in crate::backend::wayland) use completion::{ - ClipboardOperationController, ClipboardOperationIdSource, ClipboardPoll, + ClipboardOperationController, ClipboardOperationIdSource, ClipboardPoll, ClipboardSubmitFailure, }; pub(super) const WAYSCRIBER_SELECTION_MIME: &str = "application/vnd.wayscriber.selection+json"; diff --git a/src/backend/wayland/state.rs b/src/backend/wayland/state.rs index 66b23a203..b997cc632 100644 --- a/src/backend/wayland/state.rs +++ b/src/backend/wayland/state.rs @@ -62,6 +62,7 @@ use crate::{ types::CaptureType, }, config::{Action, Config}, + desktop_open::DesktopOpenRequest, input::state::{ClipboardPasteRequest, TextClipboardRequest, TextPasteTarget}, input::{DrawingState, EraserMode, InputState, Key, Tool, ZoomAction}, session::SessionOptions, @@ -101,6 +102,7 @@ mod clipboard; mod color_picker; mod core; mod data; +mod desktop_open; mod eyedropper; mod gtk_toolbar; mod helpers; @@ -254,6 +256,9 @@ pub(super) struct WaylandState { pub(super) clipboard_paste: ClipboardOperationController, pub(super) clipboard_hex_copy: ClipboardOperationController>, + /// Desktop-open work completes off-dispatch; successful completion is what + /// requests overlay exit, so runtime-owned broker teardown cannot race it. + pub(super) desktop_open: ClipboardOperationController>, pub(super) pending_hex_copy: Option, /// Async wl-copy pipeline for text-editor selections (Ctrl+C / Ctrl+X). pub(super) clipboard_text_copy: diff --git a/src/backend/wayland/state/core/init.rs b/src/backend/wayland/state/core/init.rs index 252f08ce5..9715d79bf 100644 --- a/src/backend/wayland/state/core/init.rs +++ b/src/backend/wayland/state/core/init.rs @@ -114,6 +114,10 @@ impl WaylandState { clipboard_operation_ids.clone(), runtime_wake.clone(), ); + let desktop_open = ClipboardOperationController::new( + clipboard_operation_ids.clone(), + runtime_wake.clone(), + ); let clipboard_text_copy = ClipboardOperationController::new( clipboard_operation_ids.clone(), runtime_wake.clone(), @@ -148,6 +152,7 @@ impl WaylandState { clipboard_publish, clipboard_paste, clipboard_hex_copy, + desktop_open, pending_hex_copy: None, clipboard_text_copy, pending_text_copy: Default::default(), diff --git a/src/backend/wayland/state/desktop_open.rs b/src/backend/wayland/state/desktop_open.rs new file mode 100644 index 000000000..c65a6e746 --- /dev/null +++ b/src/backend/wayland/state/desktop_open.rs @@ -0,0 +1,209 @@ +//! Runtime-owned desktop-open completion. +//! +//! Input handlers record intent only. The bounded broker operation runs on a +//! worker, wakes the Wayland loop, and requests overlay exit only after the +//! opener has completed successfully. + +use std::time::Duration; + +use super::{ClipboardOperationController, WaylandState}; +use crate::backend::wayland::clipboard::{ClipboardPoll, ClipboardSubmitFailure}; +use crate::desktop_open::{DesktopOpenInvocation, DesktopOpenRequest}; +use crate::input::state::{Toast, ToastPriority}; + +enum DesktopOpenCompletion { + Pending, + Opened(DesktopOpenRequest), + Failed { + request: DesktopOpenRequest, + reason: String, + }, +} + +impl WaylandState { + pub(in crate::backend::wayland) fn handle_desktop_open(&mut self, request: DesktopOpenRequest) { + if let Err(failure) = + queue_desktop_open(&mut self.desktop_open, request, crate::desktop_open::open) + { + let (error, request) = failure.into_parts(); + self.report_desktop_open_failure(request, error.to_string()); + } + } + + pub(in crate::backend::wayland) fn poll_desktop_open_completion(&mut self) { + let completion = classify_completion(self.desktop_open.poll()); + apply_exit_policy(&completion, &mut self.input_state.should_exit); + match completion { + DesktopOpenCompletion::Pending => {} + DesktopOpenCompletion::Opened(request) => { + log::info!( + "Opened {} at {}", + request.target_name(), + request.path().display() + ); + } + DesktopOpenCompletion::Failed { request, reason } => { + self.report_desktop_open_failure(request, reason); + } + } + } + + pub(in crate::backend::wayland) fn desktop_open_in_progress(&self) -> bool { + self.desktop_open.is_active() + } + + fn report_desktop_open_failure(&mut self, request: DesktopOpenRequest, reason: String) { + log::warn!( + "Failed to open {} at {}: {}", + request.target_name(), + request.path().display(), + reason + ); + // If an opener partially launched an application before failing, keep + // this failure visible instead of immediately applying focus-loss exit. + self.suppress_focus_exit_for(Duration::from_millis(1500)); + self.input_state.push_toast( + ToastPriority::Critical, + "launcher", + Toast::error(request.failure_notice()), + ); + } +} + +fn queue_desktop_open( + controller: &mut ClipboardOperationController>, + request: DesktopOpenRequest, + open: impl FnOnce(&DesktopOpenInvocation) -> anyhow::Result<()> + Send + 'static, +) -> Result<(), ClipboardSubmitFailure> { + let invocation = request.invocation(); + controller + .try_submit(request, "wayscriber-desktop-open", move || { + open(&invocation).map_err(|error| format!("{error:#}")) + }) + .map(drop) +} + +fn classify_completion( + poll: ClipboardPoll>, +) -> DesktopOpenCompletion { + match poll { + ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => DesktopOpenCompletion::Pending, + ClipboardPoll::Ready { + context: request, + outcome, + .. + } => classify_result(request, outcome), + ClipboardPoll::ProducerFailed { + context: request, + reason, + .. + } => DesktopOpenCompletion::Failed { request, reason }, + ClipboardPoll::Disconnected { + context: request, .. + } => DesktopOpenCompletion::Failed { + request, + reason: "desktop-open worker disconnected".to_string(), + }, + } +} + +fn classify_result( + request: DesktopOpenRequest, + outcome: Result<(), String>, +) -> DesktopOpenCompletion { + match outcome { + Ok(()) => DesktopOpenCompletion::Opened(request), + Err(reason) => DesktopOpenCompletion::Failed { request, reason }, + } +} + +fn apply_exit_policy(completion: &DesktopOpenCompletion, should_exit: &mut bool) { + if matches!(completion, DesktopOpenCompletion::Opened(_)) { + *should_exit = true; + } +} + +#[cfg(test)] +mod tests { + use std::sync::mpsc; + use std::time::{Duration, Instant}; + + use super::*; + use crate::backend::wayland::{RuntimeWakeSource, clipboard::ClipboardOperationIdSource}; + + #[test] + fn dispatch_returns_before_helper_completion_and_exit_waits_for_success() { + let wake = RuntimeWakeSource::new().unwrap(); + let mut controller = + ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + let request = DesktopOpenRequest::CaptureFolder("/tmp/capture".into()); + let (release_tx, release_rx) = mpsc::channel(); + let fallback_release = release_tx.clone(); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(500)); + let _ = fallback_release.send(()); + }); + + let started = Instant::now(); + queue_desktop_open(&mut controller, request.clone(), move |_| { + release_rx.recv().unwrap(); + Ok(()) + }) + .unwrap(); + assert!( + started.elapsed() < Duration::from_millis(250), + "desktop-open submission blocked event dispatch" + ); + assert!(matches!( + { + let completion = classify_completion(controller.poll()); + let mut should_exit = false; + apply_exit_policy(&completion, &mut should_exit); + assert!(!should_exit); + completion + }, + DesktopOpenCompletion::Pending, + )); + + release_tx.send(()).unwrap(); + let deadline = Instant::now() + Duration::from_secs(1); + loop { + match classify_completion(controller.poll()) { + DesktopOpenCompletion::Pending => { + assert!( + Instant::now() < deadline, + "desktop-open completion was not published" + ); + std::thread::yield_now(); + } + DesktopOpenCompletion::Opened(completed) => { + assert_eq!(completed, request); + let mut should_exit = false; + apply_exit_policy(&DesktopOpenCompletion::Opened(completed), &mut should_exit); + assert!(should_exit); + break; + } + DesktopOpenCompletion::Failed { reason, .. } => { + panic!("desktop-open worker failed: {reason}"); + } + } + } + } + + #[test] + fn failed_helper_completion_never_requests_exit() { + let request = DesktopOpenRequest::ConfigFile("/tmp/config.toml".into()); + let completion = classify_result(request.clone(), Err("injected failure".to_string())); + let mut should_exit = false; + apply_exit_policy(&completion, &mut should_exit); + + assert!(matches!( + completion, + DesktopOpenCompletion::Failed { + request: failed, + reason, + } if failed == request && reason == "injected failure" + )); + assert!(!should_exit); + } +} diff --git a/src/desktop_open.rs b/src/desktop_open.rs index 8b2c18d6a..cf63470d4 100644 --- a/src/desktop_open.rs +++ b/src/desktop_open.rs @@ -5,7 +5,7 @@ //! trusted Wayscriber HTTPS host rule before they reach the broker. use std::ffi::{OsStr, OsString}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::time::Duration; use anyhow::{Context, Result, bail}; @@ -19,6 +19,38 @@ pub(crate) struct DesktopOpenInvocation { arguments: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DesktopOpenRequest { + CaptureFolder(PathBuf), + ConfigFile(PathBuf), +} + +impl DesktopOpenRequest { + pub(crate) fn invocation(&self) -> DesktopOpenInvocation { + path(self.path()) + } + + pub(crate) fn path(&self) -> &Path { + match self { + Self::CaptureFolder(path) | Self::ConfigFile(path) => path, + } + } + + pub(crate) fn target_name(&self) -> &'static str { + match self { + Self::CaptureFolder(_) => "capture folder", + Self::ConfigFile(_) => "config file", + } + } + + pub(crate) fn failure_notice(&self) -> &'static str { + match self { + Self::CaptureFolder(_) => "Failed to open capture folder.", + Self::ConfigFile(_) => "Failed to open config file.", + } + } +} + impl DesktopOpenInvocation { pub(crate) fn program(&self) -> &OsStr { &self.program @@ -34,16 +66,7 @@ impl DesktopOpenInvocation { /// cannot cancel the helper they just requested. pub(crate) fn open(invocation: &DesktopOpenInvocation) -> Result<()> { let broker = crate::process_broker::current()?; - run_with(invocation, |program, arguments, timeout, output_cap| { - broker.run( - crate::process_broker::HelperKind::DesktopOpen, - program, - arguments, - Vec::new(), - timeout, - output_cap, - ) - }) + run_with_broker(&broker, invocation) } /// Run a desktop opener without blocking the Wayland or tray callback that @@ -54,16 +77,7 @@ pub(crate) fn open_in_background(invocation: DesktopOpenInvocation) -> Result<() std::thread::Builder::new() .name("wayscriber-desktop-open".to_string()) .spawn(move || { - if let Err(err) = run_with(&invocation, |program, arguments, timeout, output_cap| { - broker.run( - crate::process_broker::HelperKind::DesktopOpen, - program, - arguments, - Vec::new(), - timeout, - output_cap, - ) - }) { + if let Err(err) = run_with_broker(&broker, &invocation) { // Do not include the target or captured output: an About report // URL can carry diagnostics in its fragment. log::warn!("Desktop opener failed: {err:#}"); @@ -73,6 +87,22 @@ pub(crate) fn open_in_background(invocation: DesktopOpenInvocation) -> Result<() Ok(()) } +fn run_with_broker( + broker: &crate::process_broker::ProcessBroker, + invocation: &DesktopOpenInvocation, +) -> Result<()> { + run_with(invocation, |program, arguments, timeout, output_cap| { + broker.run( + crate::process_broker::HelperKind::DesktopOpen, + program, + arguments, + Vec::new(), + timeout, + output_cap, + ) + }) +} + fn run_with( invocation: &DesktopOpenInvocation, run: impl FnOnce( diff --git a/src/input/state/core/base/types.rs b/src/input/state/core/base/types.rs index 1d7d4df83..176d4396b 100644 --- a/src/input/state/core/base/types.rs +++ b/src/input/state/core/base/types.rs @@ -502,6 +502,7 @@ pub enum PendingBackendAction { Screenshot(Action), CanvasExport(Action), BoardPdfExport(Action), + DesktopOpen(crate::desktop_open::DesktopOpenRequest), ClearSavedToolState, } diff --git a/src/input/state/core/utility/launcher.rs b/src/input/state/core/utility/launcher.rs index 3a319ee69..41923a106 100644 --- a/src/input/state/core/utility/launcher.rs +++ b/src/input/state/core/utility/launcher.rs @@ -2,7 +2,7 @@ use super::super::base::InputState; use crate::config::Config; use crate::configurator_destination::{ConfiguratorDestination, configurator_launch_arguments}; use crate::env_vars::CONFIGURATOR_ENV; -use crate::input::state::{Toast, ToastPriority}; +use crate::input::state::{PendingBackendAction, Toast, ToastPriority}; use std::ffi::{OsStr, OsString}; /// Launch a helper from the Wayland callback thread. @@ -42,26 +42,6 @@ fn launch_failure_message(error: &anyhow::Error, failed: &'static str) -> &'stat } } -/// Complete desktop-open before requesting runtime exit. Dropping the runtime -/// also tears down its broker, so reversing these operations would race the -/// bounded helper against broker shutdown. -fn open_before_runtime_exit( - invocation: &crate::desktop_open::DesktopOpenInvocation, - should_exit: &mut bool, -) -> anyhow::Result<()> { - open_before_runtime_exit_with(invocation, should_exit, crate::desktop_open::open) -} - -fn open_before_runtime_exit_with( - invocation: &crate::desktop_open::DesktopOpenInvocation, - should_exit: &mut bool, - open: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> anyhow::Result<()>, -) -> anyhow::Result<()> { - open(invocation)?; - *should_exit = true; - Ok(()) -} - impl InputState { /// Open the About dialog, closing the overlay first. /// @@ -137,7 +117,7 @@ impl InputState { // transport to do it. if !launch_deferred_by_busy_broker(&err) && self.open_config_file_default() { log::info!( - "Opened config file with default application because wayscriber-configurator was unavailable" + "Queued config file for the default application because wayscriber-configurator was unavailable" ); } else { self.push_toast( @@ -177,27 +157,10 @@ impl InputState { return; }; - let invocation = crate::desktop_open::path(&folder); - match open_before_runtime_exit(&invocation, &mut self.should_exit) { - Ok(()) => { - log::info!("Opened capture folder at {}", folder.display()); - } - Err(err) => { - log::error!( - "Failed to open capture folder at {}: {}", - folder.display(), - err - ); - self.push_toast( - ToastPriority::Critical, - "launcher", - Toast::error(launch_failure_message( - &err, - "Failed to open capture folder.", - )), - ); - } - } + log::info!("Queued capture folder open at {}", folder.display()); + self.set_pending_backend_action(PendingBackendAction::DesktopOpen( + crate::desktop_open::DesktopOpenRequest::CaptureFolder(folder), + )); } /// Opens the primary config file using the desktop default application. @@ -215,59 +178,10 @@ impl InputState { } }; - let invocation = crate::desktop_open::path(&path); - match open_before_runtime_exit(&invocation, &mut self.should_exit) { - Ok(()) => { - log::info!("Opened config file at {}", path.display()); - true - } - Err(err) => { - log::error!("Failed to open config file at {}: {}", path.display(), err); - self.push_toast( - ToastPriority::Critical, - "launcher", - Toast::error(launch_failure_message(&err, "Failed to open config file.")), - ); - false - } - } - } -} - -#[cfg(test)] -mod tests { - use std::cell::Cell; - use std::path::Path; - - use super::*; - - #[test] - fn desktop_open_must_complete_before_runtime_exit_is_requested() { - let invocation = crate::desktop_open::path(Path::new("/tmp/capture")); - let completed = Cell::new(false); - let mut should_exit = false; - - open_before_runtime_exit_with(&invocation, &mut should_exit, |_| { - completed.set(true); - Ok(()) - }) - .unwrap(); - - assert!(completed.get()); - assert!(should_exit); - } - - #[test] - fn failed_desktop_open_keeps_the_runtime_and_broker_owner_alive() { - let invocation = crate::desktop_open::path(Path::new("/tmp/capture")); - let mut should_exit = false; - - let error = open_before_runtime_exit_with(&invocation, &mut should_exit, |_| { - Err(anyhow::anyhow!("injected desktop-open failure")) - }) - .unwrap_err(); - - assert!(error.to_string().contains("injected desktop-open failure")); - assert!(!should_exit); + log::info!("Queued config file open at {}", path.display()); + self.set_pending_backend_action(PendingBackendAction::DesktopOpen( + crate::desktop_open::DesktopOpenRequest::ConfigFile(path), + )); + true } } diff --git a/src/input/state/core/utility/pending.rs b/src/input/state/core/utility/pending.rs index f2b914bfd..2c74ebb89 100644 --- a/src/input/state/core/utility/pending.rs +++ b/src/input/state/core/utility/pending.rs @@ -242,6 +242,20 @@ mod tests { assert_eq!(state.take_pending_backend_action(), None); } + #[test] + fn pending_desktop_open_is_taken_without_requesting_early_exit() { + let mut state = make_state(); + let request = crate::desktop_open::DesktopOpenRequest::CaptureFolder("/tmp/capture".into()); + state.set_pending_backend_action(PendingBackendAction::DesktopOpen(request.clone())); + + assert!(!state.should_exit); + assert_eq!( + state.take_pending_backend_action(), + Some(PendingBackendAction::DesktopOpen(request)) + ); + assert_eq!(state.take_pending_backend_action(), None); + } + #[test] fn pending_output_focus_action_is_taken_once() { let mut state = make_state(); diff --git a/src/process_broker/mod.rs b/src/process_broker/mod.rs index fa7c998cc..ee79023ea 100644 --- a/src/process_broker/mod.rs +++ b/src/process_broker/mod.rs @@ -15,6 +15,6 @@ mod wire; #[cfg(test)] mod tests; -pub(crate) use client::{BROKER_BUSY, BrokerChild, current, start_for_runtime}; +pub(crate) use client::{BROKER_BUSY, BrokerChild, ProcessBroker, current, start_for_runtime}; pub(crate) use server::run_internal_broker_if_requested; pub(crate) use wire::{BrokerOutput, HelperKind, HelperLifetime, STDOUT_CAP_EXCEEDED}; From 95b89652f9d6ada3bfa6081757f074750ebfe665 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:40:17 +0200 Subject: [PATCH 09/10] fix: preserve desktop-open across focus transfer --- src/backend/wayland/clipboard/mod.rs | 5 - src/backend/wayland/handlers/keyboard/mod.rs | 117 ++++++++-- src/backend/wayland/mod.rs | 5 + .../completion.rs => runtime_operation.rs} | 204 ++++++++++-------- src/backend/wayland/state.rs | 18 +- src/backend/wayland/state/clipboard.rs | 27 +-- src/backend/wayland/state/color_picker.rs | 42 ++-- src/backend/wayland/state/core/init.rs | 34 ++- src/backend/wayland/state/desktop_open.rs | 32 ++- src/backend/wayland/state/text_clipboard.rs | 55 ++--- 10 files changed, 322 insertions(+), 217 deletions(-) rename src/backend/wayland/{clipboard/completion.rs => runtime_operation.rs} (73%) diff --git a/src/backend/wayland/clipboard/mod.rs b/src/backend/wayland/clipboard/mod.rs index 6fcb05df6..888d18214 100644 --- a/src/backend/wayland/clipboard/mod.rs +++ b/src/backend/wayland/clipboard/mod.rs @@ -11,15 +11,10 @@ pub(in crate::backend::wayland) use transfer::{ FailedLocalSelectionProbe, PasteAction, TransferEffect, TransferPlan, TransferWarning, }; -mod completion; mod file_list; mod image; mod system; pub(in crate::backend::wayland) mod transfer; -pub(in crate::backend::wayland) use completion::{ - ClipboardOperationController, ClipboardOperationIdSource, ClipboardPoll, ClipboardSubmitFailure, -}; - pub(super) const WAYSCRIBER_SELECTION_MIME: &str = "application/vnd.wayscriber.selection+json"; // A pasted image is persisted in the visible frame and in the Create undo action. diff --git a/src/backend/wayland/handlers/keyboard/mod.rs b/src/backend/wayland/handlers/keyboard/mod.rs index e69170d87..7ffbc85a8 100644 --- a/src/backend/wayland/handlers/keyboard/mod.rs +++ b/src/backend/wayland/handlers/keyboard/mod.rs @@ -14,6 +14,34 @@ use crate::{config::Action, input::Key, notification}; use super::super::state::WaylandState; pub(in crate::backend::wayland) use translate::keysym_to_key; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(in crate::backend::wayland) enum XdgFocusLeaveAction { + Ignore, + AwaitDesktopOpen, + RestoreClipboardFocus, + StayOpen, + Exit, +} + +pub(in crate::backend::wayland) fn xdg_focus_leave_action( + is_xdg_window: bool, + desktop_open_in_progress: bool, + focus_exit_suppressed: bool, + focus_loss_exits_overlay: bool, +) -> XdgFocusLeaveAction { + if !is_xdg_window { + XdgFocusLeaveAction::Ignore + } else if desktop_open_in_progress { + XdgFocusLeaveAction::AwaitDesktopOpen + } else if focus_exit_suppressed { + XdgFocusLeaveAction::RestoreClipboardFocus + } else if focus_loss_exits_overlay { + XdgFocusLeaveAction::Exit + } else { + XdgFocusLeaveAction::StayOpen + } +} + impl KeyboardHandler for WaylandState { fn enter( &mut self, @@ -67,30 +95,44 @@ impl KeyboardHandler for WaylandState { self.set_board_pan_key_held(false); self.stop_board_pan(); - if self.surface.is_xdg_window() && self.focus_exit_suppressed() { - warn!("Keyboard focus lost in xdg fallback; suppressing exit after clipboard action"); - self.set_xdg_close_guard_for(Duration::from_millis(2500)); - self.request_xdg_activation(qh); - return; - } - - if self.surface.is_xdg_window() { - if !self.xdg_focus_loss_exits_overlay() { + match xdg_focus_leave_action( + self.surface.is_xdg_window(), + self.desktop_open_in_progress(), + self.focus_exit_suppressed(), + self.xdg_focus_loss_exits_overlay(), + ) { + XdgFocusLeaveAction::Ignore => {} + XdgFocusLeaveAction::AwaitDesktopOpen => { + // The opener deliberately transfers focus. Its completion owns + // overlay exit so the broker guard cannot be dropped mid-run. + warn!( + "Keyboard focus left the xdg fallback during desktop-open; awaiting helper completion" + ); + } + XdgFocusLeaveAction::RestoreClipboardFocus => { + warn!( + "Keyboard focus lost in xdg fallback; suppressing exit after clipboard action" + ); + self.set_xdg_close_guard_for(Duration::from_millis(2500)); + self.request_xdg_activation(qh); + } + XdgFocusLeaveAction::StayOpen => { warn!( "Keyboard focus lost in xdg fallback; keeping overlay open without auto-reactivation (ui.xdg_focus_loss_behavior=stay)" ); self.set_xdg_close_guard_for(Duration::from_millis(2500)); - return; } - warn!("Keyboard focus lost in xdg fallback; exiting overlay"); - notification::send_notification_async( - &self.tokio_handle, - "Wayscriber lost focus".to_string(), - "The desktop could not keep the overlay focused, so Wayscriber closed it." - .to_string(), - Some("dialog-warning".to_string()), - ); - self.input_state.should_exit = true; + XdgFocusLeaveAction::Exit => { + warn!("Keyboard focus lost in xdg fallback; exiting overlay"); + notification::send_notification_async( + &self.tokio_handle, + "Wayscriber lost focus".to_string(), + "The desktop could not keep the overlay focused, so Wayscriber closed it." + .to_string(), + Some("dialog-warning".to_string()), + ); + self.input_state.should_exit = true; + } } } @@ -469,4 +511,41 @@ mod tests { assert!(!is_repeatable_key(Key::Tab)); assert!(!is_repeatable_key(Key::F10)); } + + #[test] + fn active_desktop_open_owns_xdg_focus_leave_for_every_focus_policy() { + for focus_exit_suppressed in [false, true] { + for focus_loss_exits_overlay in [false, true] { + assert_eq!( + xdg_focus_leave_action( + true, + true, + focus_exit_suppressed, + focus_loss_exits_overlay, + ), + XdgFocusLeaveAction::AwaitDesktopOpen, + ); + } + } + } + + #[test] + fn focus_leave_routing_preserves_non_desktop_open_policies() { + assert_eq!( + xdg_focus_leave_action(false, true, true, true), + XdgFocusLeaveAction::Ignore, + ); + assert_eq!( + xdg_focus_leave_action(true, false, true, true), + XdgFocusLeaveAction::RestoreClipboardFocus, + ); + assert_eq!( + xdg_focus_leave_action(true, false, false, false), + XdgFocusLeaveAction::StayOpen, + ); + assert_eq!( + xdg_focus_leave_action(true, false, false, true), + XdgFocusLeaveAction::Exit, + ); + } } diff --git a/src/backend/wayland/mod.rs b/src/backend/wayland/mod.rs index ba87c5fee..304fc8c36 100644 --- a/src/backend/wayland/mod.rs +++ b/src/backend/wayland/mod.rs @@ -9,6 +9,7 @@ pub(crate) mod input_monitor; mod overlay_passthrough; mod portal_capture; mod portal_task; +mod runtime_operation; mod runtime_ui_state; mod session; mod state; @@ -34,5 +35,9 @@ mod zoom; pub use backend::WaylandBackend; pub(crate) use backend::runtime_wake::{RuntimeWakeHandle, RuntimeWakeSource}; +pub(in crate::backend::wayland) use runtime_operation::{ + RuntimeOperationController, RuntimeOperationIdSource, RuntimeOperationPoll, + RuntimeOperationSubmitFailure, +}; #[cfg(feature = "tablet-input")] pub use tablet_types::TabletToolType; diff --git a/src/backend/wayland/clipboard/completion.rs b/src/backend/wayland/runtime_operation.rs similarity index 73% rename from src/backend/wayland/clipboard/completion.rs rename to src/backend/wayland/runtime_operation.rs index d5caf6ac4..034b20bb3 100644 --- a/src/backend/wayland/clipboard/completion.rs +++ b/src/backend/wayland/runtime_operation.rs @@ -1,4 +1,4 @@ -//! Identified capacity-one completion transport for event-loop clipboard operations. +//! Identified capacity-one completion transport for event-loop runtime operations. use std::fmt; use std::panic::{AssertUnwindSafe, catch_unwind}; @@ -8,124 +8,127 @@ use std::sync::{Arc, Mutex}; use crate::backend::wayland::RuntimeWakeHandle; #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub(in crate::backend::wayland) struct ClipboardOperationId(u64); +pub(in crate::backend::wayland) struct RuntimeOperationId(u64); -impl fmt::Display for ClipboardOperationId { +impl fmt::Display for RuntimeOperationId { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(formatter) } } #[derive(Clone)] -pub(in crate::backend::wayland) struct ClipboardOperationIdSource { +pub(in crate::backend::wayland) struct RuntimeOperationIdSource { next: Arc>>, } -impl ClipboardOperationIdSource { +impl RuntimeOperationIdSource { pub(in crate::backend::wayland) fn new() -> Self { Self { next: Arc::new(Mutex::new(Some(1))), } } - fn allocate(&self) -> Result { + fn allocate(&self) -> Result { let mut next = self .next .lock() - .map_err(|_| ClipboardSubmitError::Unhealthy)?; - let value = next.ok_or(ClipboardSubmitError::IdentityExhausted)?; + .map_err(|_| RuntimeOperationSubmitError::Unhealthy)?; + let value = next.ok_or(RuntimeOperationSubmitError::IdentityExhausted)?; *next = value.checked_add(1); - Ok(ClipboardOperationId(value)) + Ok(RuntimeOperationId(value)) } } #[derive(Debug, Clone, PartialEq, Eq)] -pub(in crate::backend::wayland) enum ClipboardSubmitError { - Busy { active_id: ClipboardOperationId }, +pub(in crate::backend::wayland) enum RuntimeOperationSubmitError { + Busy { active_id: RuntimeOperationId }, IdentityExhausted, Unhealthy, SpawnFailed { reason: String }, } -impl fmt::Display for ClipboardSubmitError { +impl fmt::Display for RuntimeOperationSubmitError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Busy { active_id } => { - write!(formatter, "clipboard operation {active_id} is still active") + write!(formatter, "runtime operation {active_id} is still active") } - Self::IdentityExhausted => formatter.write_str("clipboard operation IDs exhausted"), - Self::Unhealthy => formatter.write_str("clipboard completion controller is unhealthy"), + Self::IdentityExhausted => formatter.write_str("runtime operation IDs exhausted"), + Self::Unhealthy => formatter.write_str("runtime operation controller is unhealthy"), Self::SpawnFailed { reason } => { - write!(formatter, "failed to spawn clipboard producer: {reason}") + write!( + formatter, + "failed to spawn runtime operation worker: {reason}" + ) } } } } #[derive(Debug)] -pub(in crate::backend::wayland) struct ClipboardSubmitFailure { - error: ClipboardSubmitError, +pub(in crate::backend::wayland) struct RuntimeOperationSubmitFailure { + error: RuntimeOperationSubmitError, context: C, } -impl ClipboardSubmitFailure { - pub(in crate::backend::wayland) fn into_parts(self) -> (ClipboardSubmitError, C) { +impl RuntimeOperationSubmitFailure { + pub(in crate::backend::wayland) fn into_parts(self) -> (RuntimeOperationSubmitError, C) { (self.error, self.context) } } #[derive(Debug, PartialEq, Eq)] -pub(in crate::backend::wayland) enum ClipboardPoll { +pub(in crate::backend::wayland) enum RuntimeOperationPoll { Idle, Pending { - id: ClipboardOperationId, + id: RuntimeOperationId, }, Ready { - id: ClipboardOperationId, + id: RuntimeOperationId, context: C, outcome: T, }, ProducerFailed { - id: ClipboardOperationId, + id: RuntimeOperationId, context: C, reason: String, }, Disconnected { - id: ClipboardOperationId, + id: RuntimeOperationId, context: C, }, } enum ProducerMessage { Ready { - id: ClipboardOperationId, + id: RuntimeOperationId, outcome: T, }, Failed { - id: ClipboardOperationId, + id: RuntimeOperationId, reason: String, }, } struct ActiveOperation { - id: ClipboardOperationId, + id: RuntimeOperationId, context: C, receiver: Receiver>, } -pub(in crate::backend::wayland) struct ClipboardOperationController { - ids: ClipboardOperationIdSource, +pub(in crate::backend::wayland) struct RuntimeOperationController { + ids: RuntimeOperationIdSource, runtime_wake: RuntimeWakeHandle, active: Option>, healthy: bool, } -impl ClipboardOperationController +impl RuntimeOperationController where T: Send + 'static, { pub(in crate::backend::wayland) fn new( - ids: ClipboardOperationIdSource, + ids: RuntimeOperationIdSource, runtime_wake: RuntimeWakeHandle, ) -> Self { Self { @@ -145,7 +148,7 @@ where context: C, thread_name: &'static str, operation: impl FnOnce() -> T + Send + 'static, - ) -> Result> { + ) -> Result> { self.try_submit_with_spawner(context, operation, |job| { std::thread::Builder::new() .name(thread_name.to_string()) @@ -159,16 +162,16 @@ where context: C, operation: impl FnOnce() -> T + Send + 'static, spawn: impl FnOnce(Box) -> std::io::Result<()>, - ) -> Result> { + ) -> Result> { if !self.healthy { - return Err(ClipboardSubmitFailure { - error: ClipboardSubmitError::Unhealthy, + return Err(RuntimeOperationSubmitFailure { + error: RuntimeOperationSubmitError::Unhealthy, context, }); } if let Some(active) = &self.active { - return Err(ClipboardSubmitFailure { - error: ClipboardSubmitError::Busy { + return Err(RuntimeOperationSubmitFailure { + error: RuntimeOperationSubmitError::Busy { active_id: active.id, }, context, @@ -177,16 +180,16 @@ where let id = match self.ids.allocate() { Ok(id) => id, Err(error) => { - if error == ClipboardSubmitError::Unhealthy { + if error == RuntimeOperationSubmitError::Unhealthy { self.healthy = false; } - return Err(ClipboardSubmitFailure { error, context }); + return Err(RuntimeOperationSubmitFailure { error, context }); } }; let (sender, receiver) = std::sync::mpsc::sync_channel(1); let runtime_wake = self.runtime_wake.clone(); let job = Box::new(move || { - let guard = ClipboardProducerExitGuard::new(id, sender, runtime_wake); + let guard = RuntimeOperationExitGuard::new(id, sender, runtime_wake); let message = match catch_unwind(AssertUnwindSafe(operation)) { Ok(outcome) => ProducerMessage::Ready { id, outcome }, Err(payload) => ProducerMessage::Failed { @@ -197,8 +200,8 @@ where guard.publish(message); }); if let Err(err) = spawn(job) { - return Err(ClipboardSubmitFailure { - error: ClipboardSubmitError::SpawnFailed { + return Err(RuntimeOperationSubmitFailure { + error: RuntimeOperationSubmitError::SpawnFailed { reason: err.to_string(), }, context, @@ -212,27 +215,29 @@ where Ok(id) } - pub(in crate::backend::wayland) fn poll(&mut self) -> ClipboardPoll { + pub(in crate::backend::wayland) fn poll(&mut self) -> RuntimeOperationPoll { let Some(active) = self.active.take() else { - return ClipboardPoll::Idle; + return RuntimeOperationPoll::Idle; }; let active_id = active.id; match active.receiver.try_recv() { Err(TryRecvError::Empty) => { self.active = Some(active); - ClipboardPoll::Pending { id: active_id } + RuntimeOperationPoll::Pending { id: active_id } } - Err(TryRecvError::Disconnected) => ClipboardPoll::Disconnected { + Err(TryRecvError::Disconnected) => RuntimeOperationPoll::Disconnected { id: active.id, context: active.context, }, - Ok(ProducerMessage::Ready { id, outcome }) if id == active_id => ClipboardPoll::Ready { - id, - context: active.context, - outcome, - }, + Ok(ProducerMessage::Ready { id, outcome }) if id == active_id => { + RuntimeOperationPoll::Ready { + id, + context: active.context, + outcome, + } + } Ok(ProducerMessage::Failed { id, reason }) if id == active_id => { - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id, context: active.context, reason, @@ -240,11 +245,11 @@ where } Ok(ProducerMessage::Ready { id, .. } | ProducerMessage::Failed { id, .. }) => { self.healthy = false; - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id: active.id, context: active.context, reason: format!( - "clipboard producer reported transport identity {id}, expected {}", + "runtime operation worker reported transport identity {id}, expected {}", active.id ), } @@ -259,20 +264,20 @@ fn panic_reason(payload: Box) -> String { } else if let Some(message) = payload.downcast_ref::() { message.clone() } else { - "clipboard producer panicked with a non-string payload".to_string() + "runtime operation worker panicked with a non-string payload".to_string() } } -struct ClipboardProducerExitGuard { - id: ClipboardOperationId, +struct RuntimeOperationExitGuard { + id: RuntimeOperationId, sender: Option>>, runtime_wake: RuntimeWakeHandle, terminal_published: bool, } -impl ClipboardProducerExitGuard { +impl RuntimeOperationExitGuard { fn new( - id: ClipboardOperationId, + id: RuntimeOperationId, sender: SyncSender>, runtime_wake: RuntimeWakeHandle, ) -> Self { @@ -288,28 +293,28 @@ impl ClipboardProducerExitGuard { let sender = self .sender .take() - .expect("clipboard producer still holds its sender until publish"); + .expect("runtime operation worker still holds its sender until publish"); let result = sender.try_send(message); self.terminal_published = true; match result { Ok(()) | Err(TrySendError::Disconnected(_)) => {} Err(TrySendError::Full(_)) => { log::error!( - "Clipboard producer {} found an impossible full terminal channel", + "Runtime operation worker {} found an impossible full terminal channel", self.id ); } } if let Err(err) = self.runtime_wake.wake() { log::error!( - "Failed to wake runtime for clipboard operation {}: {err}", + "Failed to wake runtime for runtime operation {}: {err}", self.id ); } } } -impl Drop for ClipboardProducerExitGuard { +impl Drop for RuntimeOperationExitGuard { fn drop(&mut self) { if self.terminal_published { return; @@ -318,7 +323,7 @@ impl Drop for ClipboardProducerExitGuard { self.sender.take(); if let Err(err) = self.runtime_wake.wake() { log::error!( - "Failed to wake runtime for disconnected clipboard operation {}: {err}", + "Failed to wake runtime for disconnected runtime operation {}: {err}", self.id ); } @@ -335,11 +340,10 @@ mod tests { use super::*; use crate::backend::wayland::RuntimeWakeSource; - fn controller() -> (RuntimeWakeSource, ClipboardOperationController) - { + fn controller() -> (RuntimeWakeSource, RuntimeOperationController) { let wake = RuntimeWakeSource::new().unwrap(); let controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); (wake, controller) } @@ -373,14 +377,14 @@ mod tests { #[test] fn shared_source_allocates_distinct_ids_across_controllers() { let wake = RuntimeWakeSource::new().unwrap(); - let ids = ClipboardOperationIdSource::new(); - let mut publish = ClipboardOperationController::new(ids.clone(), wake.handle()); - let mut paste = ClipboardOperationController::new(ids, wake.handle()); + let ids = RuntimeOperationIdSource::new(); + let mut publish = RuntimeOperationController::new(ids.clone(), wake.handle()); + let mut paste = RuntimeOperationController::new(ids, wake.handle()); let publish_id = publish.try_submit("publish", "test-publish", || 1).unwrap(); let paste_id = paste.try_submit("paste", "test-paste", || 2).unwrap(); - assert_eq!(publish_id, ClipboardOperationId(1)); - assert_eq!(paste_id, ClipboardOperationId(2)); + assert_eq!(publish_id, RuntimeOperationId(1)); + assert_eq!(paste_id, RuntimeOperationId(2)); } #[test] @@ -393,11 +397,11 @@ mod tests { .unwrap_err(); assert_eq!( failure.into_parts().0, - ClipboardSubmitError::Busy { active_id: first } + RuntimeOperationSubmitError::Busy { active_id: first } ); assert_eq!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { id: first, context: "first", outcome: 7, @@ -423,12 +427,18 @@ mod tests { .unwrap_err(); assert_eq!( failure.into_parts(), - (ClipboardSubmitError::Busy { active_id: first }, "second",) + ( + RuntimeOperationSubmitError::Busy { active_id: first }, + "second", + ) ); release_tx.send(()).unwrap(); wait_for_wake(&wake); - assert!(matches!(controller.poll(), ClipboardPoll::Ready { .. })); + assert!(matches!( + controller.poll(), + RuntimeOperationPoll::Ready { .. } + )); } #[test] @@ -438,7 +448,7 @@ mod tests { wait_for_wake(&wake); assert_eq!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { id, context: 11, outcome: 29, @@ -469,7 +479,7 @@ mod tests { poller.join().unwrap(); assert_eq!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { id, context: 5, outcome: 13, @@ -486,7 +496,7 @@ mod tests { wait_for_wake(&wake); assert_eq!( controller.poll(), - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id, context: 17, reason: "expected producer panic".to_string(), @@ -497,20 +507,20 @@ mod tests { #[test] fn exit_guard_disconnects_before_waking() { let wake = RuntimeWakeSource::new().unwrap(); - let ids = ClipboardOperationIdSource::new(); - let mut controller = ClipboardOperationController::::new(ids, wake.handle()); - let id = ClipboardOperationId(9); + let ids = RuntimeOperationIdSource::new(); + let mut controller = RuntimeOperationController::::new(ids, wake.handle()); + let id = RuntimeOperationId(9); let (sender, receiver) = std::sync::mpsc::sync_channel(1); controller.active = Some(ActiveOperation { id, context: 31, receiver, }); - drop(ClipboardProducerExitGuard::new(id, sender, wake.handle())); + drop(RuntimeOperationExitGuard::new(id, sender, wake.handle())); wait_for_wake(&wake); assert_eq!( controller.poll(), - ClipboardPoll::Disconnected { id, context: 31 } + RuntimeOperationPoll::Disconnected { id, context: 31 } ); } @@ -527,7 +537,7 @@ mod tests { assert_eq!( failure.into_parts(), ( - ClipboardSubmitError::SpawnFailed { + RuntimeOperationSubmitError::SpawnFailed { reason: "injected spawn failure".to_string(), }, 41, @@ -545,14 +555,14 @@ mod tests { controller .try_submit(42, "test-after-spawn-failure", || 2) .unwrap(), - ClipboardOperationId(2) + RuntimeOperationId(2) ); } #[test] fn identity_mismatch_restores_active_context_and_disables_controller() { let (_wake, mut controller) = controller::(); - let active_id = ClipboardOperationId(3); + let active_id = RuntimeOperationId(3); let (sender, receiver) = std::sync::mpsc::sync_channel(1); controller.active = Some(ActiveOperation { id: active_id, @@ -561,13 +571,13 @@ mod tests { }); sender .try_send(ProducerMessage::Ready { - id: ClipboardOperationId(4), + id: RuntimeOperationId(4), outcome: 99, }) .unwrap(); assert!(matches!( controller.poll(), - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id, context: 43, .. @@ -576,7 +586,10 @@ mod tests { let failure = controller .try_submit(44, "test-unhealthy", || 1) .unwrap_err(); - assert_eq!(failure.into_parts().0, ClipboardSubmitError::Unhealthy); + assert_eq!( + failure.into_parts().0, + RuntimeOperationSubmitError::Unhealthy + ); } #[test] @@ -584,15 +597,18 @@ mod tests { let (wake, mut controller) = controller::(); *controller.ids.next.lock().unwrap() = Some(u64::MAX); let id = controller.try_submit(1, "test-max-id", || 2).unwrap(); - assert_eq!(id, ClipboardOperationId(u64::MAX)); + assert_eq!(id, RuntimeOperationId(u64::MAX)); wait_for_wake(&wake); - assert!(matches!(controller.poll(), ClipboardPoll::Ready { .. })); + assert!(matches!( + controller.poll(), + RuntimeOperationPoll::Ready { .. } + )); let failure = controller .try_submit(3, "test-exhausted", || 4) .unwrap_err(); assert_eq!( failure.into_parts().0, - ClipboardSubmitError::IdentityExhausted + RuntimeOperationSubmitError::IdentityExhausted ); } diff --git a/src/backend/wayland/state.rs b/src/backend/wayland/state.rs index b997cc632..0b1e2aaaf 100644 --- a/src/backend/wayland/state.rs +++ b/src/backend/wayland/state.rs @@ -74,11 +74,9 @@ pub use self::data::{ MoveDragKind, OverlaySuppression, OverlaySuppressionKeyboardPolicy, XdgFrozenFullscreenState, }; use super::{ + RuntimeOperationController, RuntimeOperationIdSource, capture::{CapturePreflightRequest, CaptureState, PendingPdfExport}, - clipboard::{ - ClipboardOperationController, ClipboardOperationIdSource, ClipboardPasteCompletion, - ClipboardPublishCompletion, - }, + clipboard::{ClipboardPasteCompletion, ClipboardPublishCompletion}, frozen::{ExtImageCopyManagers, FrozenState}, overlay_passthrough::set_surface_clickthrough, session::SessionState, @@ -252,21 +250,21 @@ pub(super) struct WaylandState { /// remember, so the reader can never be left running for a HUD that is /// already off. pub(super) last_input_hud_request: Option<(bool, crate::config::InputHudMode)>, - pub(super) clipboard_publish: ClipboardOperationController, + pub(super) clipboard_publish: RuntimeOperationController, pub(super) clipboard_paste: - ClipboardOperationController, - pub(super) clipboard_hex_copy: ClipboardOperationController>, + RuntimeOperationController, + pub(super) clipboard_hex_copy: RuntimeOperationController>, /// Desktop-open work completes off-dispatch; successful completion is what /// requests overlay exit, so runtime-owned broker teardown cannot race it. - pub(super) desktop_open: ClipboardOperationController>, + pub(super) desktop_open: RuntimeOperationController>, pub(super) pending_hex_copy: Option, /// Async wl-copy pipeline for text-editor selections (Ctrl+C / Ctrl+X). pub(super) clipboard_text_copy: - ClipboardOperationController>, + RuntimeOperationController>, pub(super) pending_text_copy: VecDeque, /// Async wl-paste pipeline for text-editor paste requests (Ctrl+V). pub(super) clipboard_text_paste: - ClipboardOperationController, String>>, + RuntimeOperationController, String>>, /// Text paste requests waiting behind an active read. Repeated requests in /// the current edit generation remain distinct; a new generation replaces /// stale queued requests from the old edit session. diff --git a/src/backend/wayland/state/clipboard.rs b/src/backend/wayland/state/clipboard.rs index 51246cd59..cfd4d40c9 100644 --- a/src/backend/wayland/state/clipboard.rs +++ b/src/backend/wayland/state/clipboard.rs @@ -1,10 +1,13 @@ //! Wayland-state glue for clipboard publish and paste requests. use super::WaylandState; -use crate::backend::wayland::clipboard::{ - self, ClipboardPasteCompletion, ClipboardPasteResult, ClipboardPoll, - ClipboardPublishCompletion, FailedLocalSelectionProbe, PasteAction, TransferEffect, - TransferPlan, TransferWarning, transfer, +use crate::backend::wayland::{ + RuntimeOperationPoll, + clipboard::{ + self, ClipboardPasteCompletion, ClipboardPasteResult, ClipboardPublishCompletion, + FailedLocalSelectionProbe, PasteAction, TransferEffect, TransferPlan, TransferWarning, + transfer, + }, }; use crate::input::state::ClipboardPasteRequest; use crate::input::state::{Toast, ToastPriority}; @@ -32,8 +35,8 @@ impl WaylandState { pub(in crate::backend::wayland) fn poll_clipboard_publish_completion(&mut self) { match self.clipboard_publish.poll() { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => {} - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => {} + RuntimeOperationPoll::Ready { id, context: generation, outcome, @@ -50,7 +53,7 @@ impl WaylandState { ); } } - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id, context: generation, reason, @@ -60,7 +63,7 @@ impl WaylandState { failed_clipboard_publish_completion(generation), ); } - ClipboardPoll::Disconnected { + RuntimeOperationPoll::Disconnected { id, context: generation, } => { @@ -74,8 +77,8 @@ impl WaylandState { pub(in crate::backend::wayland) fn poll_clipboard_paste_completion(&mut self) { match self.clipboard_paste.poll() { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => {} - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => {} + RuntimeOperationPoll::Ready { id, context: request, outcome, @@ -97,7 +100,7 @@ impl WaylandState { )); } } - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { id, context: request, reason, @@ -107,7 +110,7 @@ impl WaylandState { request, &reason, )); } - ClipboardPoll::Disconnected { + RuntimeOperationPoll::Disconnected { id, context: request, } => { diff --git a/src/backend/wayland/state/color_picker.rs b/src/backend/wayland/state/color_picker.rs index bb91b38ab..8771e38d1 100644 --- a/src/backend/wayland/state/color_picker.rs +++ b/src/backend/wayland/state/color_picker.rs @@ -1,7 +1,7 @@ //! Clipboard helpers for color hex values. -use super::{ClipboardOperationController, WaylandState}; -use crate::backend::wayland::clipboard::ClipboardPoll; +use super::{RuntimeOperationController, WaylandState}; +use crate::backend::wayland::RuntimeOperationPoll; use crate::clipboard_text::{ ClipboardTextError, copy_text_via_command, read_clipboard_text_via_command, }; @@ -34,8 +34,8 @@ impl WaylandState { pub(in crate::backend::wayland) fn poll_hex_copy_completion(&mut self) { match self.clipboard_hex_copy.poll() { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => {} - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => {} + RuntimeOperationPoll::Ready { context: hex, outcome: Ok(()), .. @@ -46,7 +46,7 @@ impl WaylandState { Toast::info(format!("Copied {hex}")), ); } - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context: hex, outcome: Err(err), .. @@ -58,7 +58,7 @@ impl WaylandState { Toast::warning("Failed to copy to clipboard"), ); } - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { context: hex, reason, .. @@ -70,7 +70,7 @@ impl WaylandState { Toast::warning("Failed to copy to clipboard"), ); } - ClipboardPoll::Disconnected { context: hex, .. } => { + RuntimeOperationPoll::Disconnected { context: hex, .. } => { log::error!("Hex copy producer disconnected for {hex}"); self.input_state.push_toast( ToastPriority::Info, @@ -171,7 +171,7 @@ impl WaylandState { } fn start_clipboard_copy( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, hex: String, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, ) -> Result<(), String> { @@ -183,7 +183,7 @@ fn start_clipboard_copy( } pub(super) fn queue_latest_clipboard_copy( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, pending: &mut Option, hex: String, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, @@ -193,7 +193,7 @@ pub(super) fn queue_latest_clipboard_copy( } pub(super) fn submit_pending_clipboard_copy_if_idle( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, pending: &mut Option, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, ) -> Result<(), String> { @@ -212,14 +212,14 @@ mod tests { use std::time::Duration; use super::*; + use crate::backend::wayland::RuntimeOperationIdSource; use crate::backend::wayland::RuntimeWakeSource; - use crate::backend::wayland::clipboard::ClipboardOperationIdSource; #[test] fn hex_copy_submission_stays_off_the_event_thread_until_completion() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); @@ -234,7 +234,10 @@ mod tests { .unwrap(); started_rx.recv_timeout(Duration::from_secs(1)).unwrap(); - assert!(matches!(controller.poll(), ClipboardPoll::Pending { .. })); + assert!(matches!( + controller.poll(), + RuntimeOperationPoll::Pending { .. } + )); release_tx.send(()).unwrap(); assert!( wake.wait_readable(Some(Duration::from_secs(1))).unwrap(), @@ -242,7 +245,7 @@ mod tests { ); assert!(matches!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context, outcome: Ok(()), .. @@ -254,7 +257,7 @@ mod tests { fn active_hex_copy_retains_only_the_newest_pending_request() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let mut pending = None; let (first_started_tx, first_started_rx) = mpsc::channel(); let (first_release_tx, first_release_rx) = mpsc::channel(); @@ -292,13 +295,16 @@ mod tests { ) .unwrap(); assert_eq!(pending.as_deref(), Some("#333333")); - assert!(matches!(controller.poll(), ClipboardPoll::Pending { .. })); + assert!(matches!( + controller.poll(), + RuntimeOperationPoll::Pending { .. } + )); first_release_tx.send(()).unwrap(); assert!(wake.wait_readable(Some(Duration::from_secs(1))).unwrap()); assert!(matches!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context, outcome: Ok(()), .. @@ -327,7 +333,7 @@ mod tests { assert!(wake.wait_readable(Some(Duration::from_secs(1))).unwrap()); assert!(matches!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context, outcome: Ok(()), .. diff --git a/src/backend/wayland/state/core/init.rs b/src/backend/wayland/state/core/init.rs index 9715d79bf..ffb1e5d5f 100644 --- a/src/backend/wayland/state/core/init.rs +++ b/src/backend/wayland/state/core/init.rs @@ -101,29 +101,19 @@ impl WaylandState { WaylandState::ui_animation_interval_from_fps(config.performance.ui_animation_fps); let buffer_count = config.performance.buffer_count as usize; - let clipboard_operation_ids = ClipboardOperationIdSource::new(); - let clipboard_publish = ClipboardOperationController::new( - clipboard_operation_ids.clone(), - runtime_wake.clone(), - ); - let clipboard_paste = ClipboardOperationController::new( - clipboard_operation_ids.clone(), - runtime_wake.clone(), - ); - let clipboard_hex_copy = ClipboardOperationController::new( - clipboard_operation_ids.clone(), - runtime_wake.clone(), - ); - let desktop_open = ClipboardOperationController::new( - clipboard_operation_ids.clone(), - runtime_wake.clone(), - ); - let clipboard_text_copy = ClipboardOperationController::new( - clipboard_operation_ids.clone(), - runtime_wake.clone(), - ); + let runtime_operation_ids = RuntimeOperationIdSource::new(); + let clipboard_publish = + RuntimeOperationController::new(runtime_operation_ids.clone(), runtime_wake.clone()); + let clipboard_paste = + RuntimeOperationController::new(runtime_operation_ids.clone(), runtime_wake.clone()); + let clipboard_hex_copy = + RuntimeOperationController::new(runtime_operation_ids.clone(), runtime_wake.clone()); + let desktop_open = + RuntimeOperationController::new(runtime_operation_ids.clone(), runtime_wake.clone()); + let clipboard_text_copy = + RuntimeOperationController::new(runtime_operation_ids.clone(), runtime_wake.clone()); let clipboard_text_paste = - ClipboardOperationController::new(clipboard_operation_ids, runtime_wake.clone()); + RuntimeOperationController::new(runtime_operation_ids, runtime_wake.clone()); let ocr = crate::ocr::OcrController::new(runtime_wake.clone()); Self { diff --git a/src/backend/wayland/state/desktop_open.rs b/src/backend/wayland/state/desktop_open.rs index c65a6e746..87f4623d9 100644 --- a/src/backend/wayland/state/desktop_open.rs +++ b/src/backend/wayland/state/desktop_open.rs @@ -6,8 +6,8 @@ use std::time::Duration; -use super::{ClipboardOperationController, WaylandState}; -use crate::backend::wayland::clipboard::{ClipboardPoll, ClipboardSubmitFailure}; +use super::{RuntimeOperationController, WaylandState}; +use crate::backend::wayland::{RuntimeOperationPoll, RuntimeOperationSubmitFailure}; use crate::desktop_open::{DesktopOpenInvocation, DesktopOpenRequest}; use crate::input::state::{Toast, ToastPriority}; @@ -71,10 +71,10 @@ impl WaylandState { } fn queue_desktop_open( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, request: DesktopOpenRequest, open: impl FnOnce(&DesktopOpenInvocation) -> anyhow::Result<()> + Send + 'static, -) -> Result<(), ClipboardSubmitFailure> { +) -> Result<(), RuntimeOperationSubmitFailure> { let invocation = request.invocation(); controller .try_submit(request, "wayscriber-desktop-open", move || { @@ -84,21 +84,23 @@ fn queue_desktop_open( } fn classify_completion( - poll: ClipboardPoll>, + poll: RuntimeOperationPoll>, ) -> DesktopOpenCompletion { match poll { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => DesktopOpenCompletion::Pending, - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => { + DesktopOpenCompletion::Pending + } + RuntimeOperationPoll::Ready { context: request, outcome, .. } => classify_result(request, outcome), - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { context: request, reason, .. } => DesktopOpenCompletion::Failed { request, reason }, - ClipboardPoll::Disconnected { + RuntimeOperationPoll::Disconnected { context: request, .. } => DesktopOpenCompletion::Failed { request, @@ -129,13 +131,16 @@ mod tests { use std::time::{Duration, Instant}; use super::*; - use crate::backend::wayland::{RuntimeWakeSource, clipboard::ClipboardOperationIdSource}; + use crate::backend::wayland::{ + RuntimeOperationIdSource, RuntimeWakeSource, + handlers::keyboard::{XdgFocusLeaveAction, xdg_focus_leave_action}, + }; #[test] fn dispatch_returns_before_helper_completion_and_exit_waits_for_success() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let request = DesktopOpenRequest::CaptureFolder("/tmp/capture".into()); let (release_tx, release_rx) = mpsc::channel(); let fallback_release = release_tx.clone(); @@ -154,6 +159,11 @@ mod tests { started.elapsed() < Duration::from_millis(250), "desktop-open submission blocked event dispatch" ); + assert!(controller.is_active()); + assert_eq!( + xdg_focus_leave_action(true, controller.is_active(), false, true), + XdgFocusLeaveAction::AwaitDesktopOpen, + ); assert!(matches!( { let completion = classify_completion(controller.poll()); diff --git a/src/backend/wayland/state/text_clipboard.rs b/src/backend/wayland/state/text_clipboard.rs index 2a1edea28..7c969cdfd 100644 --- a/src/backend/wayland/state/text_clipboard.rs +++ b/src/backend/wayland/state/text_clipboard.rs @@ -8,8 +8,8 @@ //! handlers, drained each input cycle, fulfill it against the compositor //! clipboard (reusing the generic clipboard worker pipeline). -use super::{ClipboardOperationController, WaylandState}; -use crate::backend::wayland::clipboard::ClipboardPoll; +use super::{RuntimeOperationController, WaylandState}; +use crate::backend::wayland::RuntimeOperationPoll; use crate::clipboard_text::{ ClipboardTextError, copy_text_via_command, read_clipboard_text_via_command, }; @@ -44,13 +44,13 @@ impl WaylandState { /// queued copy once the controller goes idle. pub(in crate::backend::wayland) fn poll_text_copy_completion(&mut self) { match self.clipboard_text_copy.poll() { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => {} - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => {} + RuntimeOperationPoll::Ready { context: request, outcome: Ok(()), .. } => self.input_state.complete_text_copy(request), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { outcome: Err(err), .. } => { log::warn!("wl-copy failed for text copy: {err}"); @@ -60,7 +60,7 @@ impl WaylandState { Toast::warning("Failed to copy to clipboard"), ); } - ClipboardPoll::ProducerFailed { reason, .. } => { + RuntimeOperationPoll::ProducerFailed { reason, .. } => { log::error!("Text copy producer failed: {reason}"); self.input_state.push_toast( ToastPriority::Info, @@ -68,7 +68,7 @@ impl WaylandState { Toast::warning("Failed to copy to clipboard"), ); } - ClipboardPoll::Disconnected { .. } => { + RuntimeOperationPoll::Disconnected { .. } => { log::error!("Text copy producer disconnected"); } } @@ -102,8 +102,8 @@ impl WaylandState { /// text-edit session. A later edit must never receive a stale completion. pub(in crate::backend::wayland) fn poll_text_paste_completion(&mut self) { match self.clipboard_text_paste.poll() { - ClipboardPoll::Idle | ClipboardPoll::Pending { .. } => {} - ClipboardPoll::Ready { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => {} + RuntimeOperationPoll::Ready { context: target, outcome, .. @@ -127,7 +127,7 @@ impl WaylandState { log::debug!("Discarding stale text clipboard paste completion"); } } - ClipboardPoll::ProducerFailed { + RuntimeOperationPoll::ProducerFailed { context: target, reason, .. @@ -137,7 +137,7 @@ impl WaylandState { self.push_text_paste_failure(); } } - ClipboardPoll::Disconnected { + RuntimeOperationPoll::Disconnected { context: target, .. } => { log::error!("Text paste producer disconnected"); @@ -178,7 +178,7 @@ impl WaylandState { } fn start_text_copy( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, request: TextClipboardRequest, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, ) -> Result<(), String> { @@ -192,7 +192,7 @@ fn start_text_copy( } fn queue_text_copy( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, pending: &mut VecDeque, request: TextClipboardRequest, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, @@ -209,7 +209,7 @@ fn queue_text_copy( } fn submit_pending_text_copy_if_idle( - controller: &mut ClipboardOperationController>, + controller: &mut RuntimeOperationController>, pending: &mut VecDeque, operation: impl FnOnce(&str) -> Result<(), String> + Send + 'static, ) -> Result<(), String> { @@ -233,7 +233,7 @@ fn read_text_paste() -> TextPasteOutcome { } fn start_text_paste( - controller: &mut ClipboardOperationController, + controller: &mut RuntimeOperationController, target: TextPasteTarget, operation: impl FnOnce() -> TextPasteOutcome + Send + 'static, ) -> Result<(), String> { @@ -244,7 +244,7 @@ fn start_text_paste( } fn queue_text_paste( - controller: &mut ClipboardOperationController, + controller: &mut RuntimeOperationController, pending: &mut VecDeque, target: TextPasteTarget, operation: impl FnOnce() -> TextPasteOutcome + Send + 'static, @@ -299,8 +299,8 @@ mod tests { use std::time::Duration; use super::*; + use crate::backend::wayland::RuntimeOperationIdSource; use crate::backend::wayland::RuntimeWakeSource; - use crate::backend::wayland::clipboard::ClipboardOperationIdSource; fn copy_request(text: &str) -> TextClipboardRequest { TextClipboardRequest { @@ -333,7 +333,7 @@ mod tests { fn text_copy_keeps_its_request_context_until_worker_completion() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); start_text_copy(&mut controller, copy_request("selected"), |text| { assert_eq!(text, "selected"); @@ -347,7 +347,7 @@ mod tests { ); assert!(matches!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context: TextClipboardRequest { text, .. }, outcome: Ok(()), .. @@ -359,7 +359,7 @@ mod tests { fn active_text_copy_retains_only_the_newest_request() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let mut pending = VecDeque::new(); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); @@ -403,7 +403,7 @@ mod tests { fn active_text_copy_preserves_every_pending_cut_request() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let mut pending = VecDeque::new(); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); @@ -450,7 +450,7 @@ mod tests { fn text_paste_read_stays_off_the_event_thread_until_completion() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); @@ -462,7 +462,10 @@ mod tests { .unwrap(); started_rx.recv_timeout(Duration::from_secs(1)).unwrap(); - assert!(matches!(controller.poll(), ClipboardPoll::Pending { .. })); + assert!(matches!( + controller.poll(), + RuntimeOperationPoll::Pending { .. } + )); release_tx.send(()).unwrap(); assert!( wake.wait_readable(Some(Duration::from_secs(1))).unwrap(), @@ -470,7 +473,7 @@ mod tests { ); assert!(matches!( controller.poll(), - ClipboardPoll::Ready { + RuntimeOperationPoll::Ready { context: TextPasteTarget { generation: 7, .. }, outcome: Ok(Some(text)), .. @@ -482,7 +485,7 @@ mod tests { fn active_text_paste_preserves_every_request_from_the_same_session() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let mut pending = VecDeque::new(); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); @@ -518,7 +521,7 @@ mod tests { fn newer_text_session_supersedes_older_pending_pastes_without_coalescing_its_own() { let wake = RuntimeWakeSource::new().unwrap(); let mut controller = - ClipboardOperationController::new(ClipboardOperationIdSource::new(), wake.handle()); + RuntimeOperationController::new(RuntimeOperationIdSource::new(), wake.handle()); let mut pending = VecDeque::new(); let (started_tx, started_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); From 9085221e8167c546aeb2d040c3fa68008d0a124b Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:45:31 +0200 Subject: [PATCH 10/10] fix: harden brokered helper handoffs Preserve helper ownership through teardown while closing argv-policy and focus-exit gaps. --- CHANGELOG.md | 19 --- src/about_window/clipboard.rs | 39 +++-- src/about_window/mod.rs | 16 +- src/about_window/state.rs | 34 +++- .../wayland/backend/event_loop/capture.rs | 4 + src/backend/wayland/backend/event_loop/mod.rs | 47 ++++-- .../wayland/clipboard/system/command.rs | 1 - src/backend/wayland/handlers/keyboard/mod.rs | 6 +- src/backend/wayland/state/desktop_open.rs | 76 ++++++--- src/capture/clipboard.rs | 1 - src/capture/sources/hyprland.rs | 18 +-- src/clipboard_text.rs | 15 +- src/daemon/tray/helpers.rs | 8 +- src/desktop_open.rs | 150 +++++------------- src/input/state/core/base/state/init.rs | 1 + src/input/state/core/base/state/structs.rs | 3 + src/input/state/core/utility/toasts.rs | 37 ++++- src/process_broker/client.rs | 2 - src/process_broker/manifest.rs | 101 ++++++++++-- src/process_broker/server.rs | 6 +- src/process_broker/tests.rs | 87 ++++++---- src/process_broker/wire.rs | 2 - 22 files changed, 398 insertions(+), 275 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index e92bcc9a8..000000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,19 +0,0 @@ -# Changelog - -## Unreleased - -### Added - -- About dialog: a "Report a problem" row. It copies your diagnostics to the clipboard and opens `https://wayscriber.com/report`, which hands them straight into a prefilled bug form. Nothing is sent automatically, and the diagnostics travel in the URL fragment, so they never reach a server log. -- Automatic guidance toasts now provide explicit "Got it" and "Tip settings…" controls. The General UI setting can disable automatic tips without disabling manual tours or real warnings. - -### Breaking (Rust source) - -- The unified top toolbar is now the only layout. Panel-era typed config fields and order groups are removed from the serde model (`side_*` placement/pin/pane keys, `show_settings_section` / mode overrides, and `ui.toolbar.items.order.{side_sections,actions,pages,boards,presets,tool_options,sessions}`). Authored values at those exact paths remain in `config.toml` as retired settings and no longer affect the overlay. Matching keys under `runtime-ui.toml`'s recognized `item_order` map are pruned on rewrite. -- Public Rust types that described the side palette / panel-only toolbar order groups are gone. Downstream crates that constructed those fields must drop them; the in-repo configurator already matches this shape. - -### Fixed - -- Stylus pressure no longer overrides the selected Marker/Textmarker or Step Marker size. Pressure-to-thickness mapping remains limited to pressure-sensitive freehand Pen strokes. -- Automatic tips and skipped-default shortcut notices now remember acknowledgement and taught-feature use instead of returning every active launch. Persistence failures are surfaced instead of causing repeat loops. -- First-run onboarding no longer requires the radial-menu flick-to-commit exercise. Saved sessions paused on that retired step continue at the reference step. diff --git a/src/about_window/clipboard.rs b/src/about_window/clipboard.rs index b048c2786..dc4d633a2 100644 --- a/src/about_window/clipboard.rs +++ b/src/about_window/clipboard.rs @@ -1,31 +1,32 @@ use anyhow::Result; use log::warn; +use std::thread::JoinHandle; -pub(super) fn open_url(url: &str) -> Result<()> { +pub(super) fn open_url(url: &str) -> Result> { open_url_with(url, |invocation| { crate::desktop_open::open_in_background(invocation.clone()) }) } -fn open_url_with( +fn open_url_with( url: &str, - open: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> Result<()>, -) -> Result<()> { - let invocation = crate::desktop_open::about_url(url)?; + open: impl FnOnce(&crate::desktop_open::DesktopOpenInvocation) -> Result, +) -> Result { + let invocation = crate::desktop_open::trusted_url(url)?; open(&invocation) } -pub(super) fn copy_text_to_clipboard(text: &str) { +pub(super) fn copy_text_to_clipboard(text: &str) -> Option> { if text.is_empty() { - return; + return None; } let text = text.to_string(); - std::thread::spawn(move || { + Some(std::thread::spawn(move || { if let Err(err) = copy_text_with_command(&text, copy_text_via_command) { warn!("Failed to copy About text to clipboard: {err:#}"); } - }); + })) } fn copy_text_with_command(text: &str, command_copy: C) -> Result<()> @@ -78,7 +79,6 @@ mod tests { for url in [ "http://wayscriber.com/report", "https://wayscriber.com.example/report", - "https://www.wayscriber.com/report", "https://example.com/report", ] { let result = open_url_with(url, |_| { @@ -91,6 +91,25 @@ mod tests { assert_eq!(spawn_calls.load(Ordering::SeqCst), 0); } + #[test] + fn open_url_accepts_manifest_trusted_www_host() { + let mut observed = None; + open_url_with( + "https://www.wayscriber.com/docs/getting-started/updating.html", + |invocation| { + observed = Some(invocation.arguments().to_vec()); + Ok(()) + }, + ) + .unwrap(); + assert_eq!( + observed.unwrap(), + [std::ffi::OsString::from( + "https://www.wayscriber.com/docs/getting-started/updating.html" + )] + ); + } + #[test] fn copy_text_with_command_short_circuits_for_empty_text() { let command_calls = AtomicUsize::new(0); diff --git a/src/about_window/mod.rs b/src/about_window/mod.rs index 15b5082f5..ed1ff3f95 100644 --- a/src/about_window/mod.rs +++ b/src/about_window/mod.rs @@ -84,8 +84,20 @@ pub fn run_about_window() -> Result<()> { plan, ); + // Join helpers on every return path so ProcessBrokerGuard teardown cannot + // cancel an in-flight Report/open/copy that already showed a success notice. + let result = run_about_event_loop(&conn, &mut event_queue, &mut state); + state.join_helper_workers(); + result +} + +fn run_about_event_loop( + conn: &Connection, + event_queue: &mut wayland_client::EventQueue, + state: &mut AboutWindowState, +) -> Result<()> { loop { - event_queue.blocking_dispatch(&mut state)?; + event_queue.blocking_dispatch(state)?; if state.should_exit { break; } @@ -134,6 +146,8 @@ struct AboutWindowState { /// Set when the update card is activated; serviced by the event loop so the /// blocking fetch never runs inside a protocol handler. check_requested: bool, + /// Open/copy workers that must finish before the process broker shuts down. + helper_workers: Vec>, content: AboutContent, plan: Plan, update: UpdateState, diff --git a/src/about_window/state.rs b/src/about_window/state.rs index 634dc85b6..23f5138c3 100644 --- a/src/about_window/state.rs +++ b/src/about_window/state.rs @@ -51,6 +51,7 @@ impl AboutWindowState { should_exit: false, needs_redraw: true, check_requested: false, + helper_workers: Vec::new(), content, plan, update, @@ -127,26 +128,36 @@ impl AboutWindowState { fn perform(&mut self, action: AboutAction) { match action { AboutAction::OpenUrl(url) => match clipboard::open_url(&url) { - Ok(()) => self.set_notice(OPENING_NOTICE), + Ok(worker) => { + self.track_helper_worker(worker); + self.set_notice(OPENING_NOTICE); + } Err(err) => { warn!("About dialog refused or failed to open a URL: {err:#}"); self.set_notice(OPEN_FAILED_NOTICE); } }, AboutAction::CopyText(text) => { - clipboard::copy_text_to_clipboard(&text); + if let Some(worker) = clipboard::copy_text_to_clipboard(&text) { + self.track_helper_worker(worker); + } self.set_notice(COPIED_NOTICE); } // Copy as well as open: the URL carries the same diagnostics in its // fragment, but a browser that never launches, or a form that drops // the prefill, still leaves them one paste away. AboutAction::ReportBug { url, diagnostics } => { - // Queue the bounded desktop-open worker before the independent - // clipboard publication worker. + // Start the desktop-open worker before the independent clipboard + // publication worker. let opened = clipboard::open_url(&url); - clipboard::copy_text_to_clipboard(&diagnostics); + if let Some(worker) = clipboard::copy_text_to_clipboard(&diagnostics) { + self.track_helper_worker(worker); + } match opened { - Ok(()) => self.set_notice(REPORTED_NOTICE), + Ok(worker) => { + self.track_helper_worker(worker); + self.set_notice(REPORTED_NOTICE); + } Err(err) => { warn!("About dialog failed to open the report URL: {err:#}"); self.set_notice(REPORT_OPEN_FAILED_NOTICE); @@ -158,6 +169,17 @@ impl AboutWindowState { } } + fn track_helper_worker(&mut self, worker: std::thread::JoinHandle<()>) { + self.helper_workers.push(worker); + } + + /// Finish in-flight open/copy workers before the process broker tears down. + pub(super) fn join_helper_workers(&mut self) { + for worker in self.helper_workers.drain(..) { + let _ = worker.join(); + } + } + fn begin_update_check(&mut self) { self.set_update(UpdateState::Checking); self.notice = None; diff --git a/src/backend/wayland/backend/event_loop/capture.rs b/src/backend/wayland/backend/event_loop/capture.rs index 8460495dd..194bbcce1 100644 --- a/src/backend/wayland/backend/event_loop/capture.rs +++ b/src/backend/wayland/backend/event_loop/capture.rs @@ -410,6 +410,10 @@ fn handle_capture_results(state: &mut WaylandState) { } } if should_exit { + // Exit-after-capture is intentional teardown. Mark it explicit so XDG + // stay-mode cannot clear should_exit while the overlay is unfocused + // (for example after a portal dialog stole focus during capture). + state.mark_xdg_explicit_close_requested(); state.input_state.should_exit = true; } } diff --git a/src/backend/wayland/backend/event_loop/mod.rs b/src/backend/wayland/backend/event_loop/mod.rs index 58fb5d5ce..a5bb0460b 100644 --- a/src/backend/wayland/backend/event_loop/mod.rs +++ b/src/backend/wayland/backend/event_loop/mod.rs @@ -194,22 +194,11 @@ pub(super) fn run_event_loop( state.process_gtk_toolbar(conn, qh); // Check immediately after dispatch returns. - if state.input_state.should_exit { - let explicit_xdg_close_requested = state.take_xdg_explicit_close_requested(); - if should_defer_xdg_unfocused_exit( - state.surface.is_xdg_window(), - !state.xdg_focus_loss_exits_overlay(), - state.has_keyboard_focus(), - explicit_xdg_close_requested, - ) { - warn!("Exit requested while unfocused in xdg stay mode; keeping overlay open"); - state.input_state.should_exit = false; - } else { - info!("Exit requested after dispatch, breaking event loop"); - break; - } + if break_on_requested_exit(state) { + break; } if state.surface.is_xdg_window() + && !state.input_state.should_exit && !state.has_keyboard_focus() && !state.desktop_open_in_progress() && state.focus_exit_suppression_expired(Instant::now()) @@ -279,6 +268,12 @@ pub(super) fn run_event_loop( .tick_radial_menu_paint(std::time::Instant::now()); capture::handle_pending_actions(state, qh); + // Desktop-open handoff (and other pending-action completions) can set + // should_exit after the post-dispatch check. Break here so exit does not + // wait on another compositor wake under a blocking dispatch timeout. + if break_on_requested_exit(state) { + break; + } state.sync_overlay_interactivity(); state.apply_onboarding_hints(); @@ -406,6 +401,27 @@ fn should_defer_xdg_unfocused_exit( is_xdg_window && stay_mode && !has_keyboard_focus && !explicit_xdg_close_requested } +fn break_on_requested_exit(state: &mut WaylandState) -> bool { + if !state.input_state.should_exit { + return false; + } + let explicit_xdg_close_requested = state.take_xdg_explicit_close_requested() + || state.input_state.take_explicit_exit_requested(); + if should_defer_xdg_unfocused_exit( + state.surface.is_xdg_window(), + !state.xdg_focus_loss_exits_overlay(), + state.has_keyboard_focus(), + explicit_xdg_close_requested, + ) { + warn!("Exit requested while unfocused in xdg stay mode; keeping overlay open"); + state.input_state.should_exit = false; + false + } else { + info!("Exit requested, breaking event loop"); + true + } +} + #[cfg(test)] mod tests { use std::cell::RefCell; @@ -437,6 +453,9 @@ mod tests { assert!(!should_defer_xdg_unfocused_exit(true, true, true, false)); assert!(!should_defer_xdg_unfocused_exit(true, false, false, false)); assert!(!should_defer_xdg_unfocused_exit(false, true, false, false)); + // Desktop-open handoff marks the close explicit so stay-mode does not + // cancel exit and reactivate over the opened application. Exit-after- + // capture uses the same explicit-close bit for the same reason. assert!(!should_defer_xdg_unfocused_exit(true, true, false, true)); } diff --git a/src/backend/wayland/clipboard/system/command.rs b/src/backend/wayland/clipboard/system/command.rs index bf21cfaab..86c12a056 100644 --- a/src/backend/wayland/clipboard/system/command.rs +++ b/src/backend/wayland/clipboard/system/command.rs @@ -51,7 +51,6 @@ impl ClipboardCommandRunner for WlClipboardCommandRunner { [OsStr::new("--type"), OsStr::new(WAYSCRIBER_SELECTION_MIME)], payload.to_vec(), timeout, - 0, ) } } diff --git a/src/backend/wayland/handlers/keyboard/mod.rs b/src/backend/wayland/handlers/keyboard/mod.rs index 7ffbc85a8..88254160f 100644 --- a/src/backend/wayland/handlers/keyboard/mod.rs +++ b/src/backend/wayland/handlers/keyboard/mod.rs @@ -103,10 +103,10 @@ impl KeyboardHandler for WaylandState { ) { XdgFocusLeaveAction::Ignore => {} XdgFocusLeaveAction::AwaitDesktopOpen => { - // The opener deliberately transfers focus. Its completion owns - // overlay exit so the broker guard cannot be dropped mid-run. + // The opener deliberately transfers focus. Overlay exit waits + // for the detached spawn handoff so teardown cannot race it. warn!( - "Keyboard focus left the xdg fallback during desktop-open; awaiting helper completion" + "Keyboard focus left the xdg fallback during desktop-open; awaiting helper handoff" ); } XdgFocusLeaveAction::RestoreClipboardFocus => { diff --git a/src/backend/wayland/state/desktop_open.rs b/src/backend/wayland/state/desktop_open.rs index 87f4623d9..6614ca6f0 100644 --- a/src/backend/wayland/state/desktop_open.rs +++ b/src/backend/wayland/state/desktop_open.rs @@ -1,8 +1,8 @@ //! Runtime-owned desktop-open completion. //! -//! Input handlers record intent only. The bounded broker operation runs on a +//! Input handlers record intent only. The detached broker spawn runs on a //! worker, wakes the Wayland loop, and requests overlay exit only after the -//! opener has completed successfully. +//! opener has been handed off successfully. use std::time::Duration; @@ -13,13 +13,34 @@ use crate::input::state::{Toast, ToastPriority}; enum DesktopOpenCompletion { Pending, - Opened(DesktopOpenRequest), + /// Broker accepted the detached spawn; the opener may still be starting. + HandedOff(DesktopOpenRequest), Failed { request: DesktopOpenRequest, reason: String, }, } +/// Overlay exit requested by a successful desktop-open handoff. +/// +/// Successful opens deliberately transfer focus, so the exit must be marked +/// explicit: xdg stay-mode otherwise cancels `should_exit` and reactivates the +/// overlay over the application the user just opened. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum HandoffExitIntent { + None, + ExitExplicitly, +} + +fn handoff_exit_intent(completion: &DesktopOpenCompletion) -> HandoffExitIntent { + match completion { + DesktopOpenCompletion::HandedOff(_) => HandoffExitIntent::ExitExplicitly, + DesktopOpenCompletion::Pending | DesktopOpenCompletion::Failed { .. } => { + HandoffExitIntent::None + } + } +} + impl WaylandState { pub(in crate::backend::wayland) fn handle_desktop_open(&mut self, request: DesktopOpenRequest) { if let Err(failure) = @@ -32,12 +53,18 @@ impl WaylandState { pub(in crate::backend::wayland) fn poll_desktop_open_completion(&mut self) { let completion = classify_completion(self.desktop_open.poll()); - apply_exit_policy(&completion, &mut self.input_state.should_exit); + match handoff_exit_intent(&completion) { + HandoffExitIntent::None => {} + HandoffExitIntent::ExitExplicitly => { + self.mark_xdg_explicit_close_requested(); + self.input_state.should_exit = true; + } + } match completion { DesktopOpenCompletion::Pending => {} - DesktopOpenCompletion::Opened(request) => { + DesktopOpenCompletion::HandedOff(request) => { log::info!( - "Opened {} at {}", + "Handed off desktop open for {} at {}", request.target_name(), request.path().display() ); @@ -114,17 +141,11 @@ fn classify_result( outcome: Result<(), String>, ) -> DesktopOpenCompletion { match outcome { - Ok(()) => DesktopOpenCompletion::Opened(request), + Ok(()) => DesktopOpenCompletion::HandedOff(request), Err(reason) => DesktopOpenCompletion::Failed { request, reason }, } } -fn apply_exit_policy(completion: &DesktopOpenCompletion, should_exit: &mut bool) { - if matches!(completion, DesktopOpenCompletion::Opened(_)) { - *should_exit = true; - } -} - #[cfg(test)] mod tests { use std::sync::mpsc; @@ -167,9 +188,7 @@ mod tests { assert!(matches!( { let completion = classify_completion(controller.poll()); - let mut should_exit = false; - apply_exit_policy(&completion, &mut should_exit); - assert!(!should_exit); + assert_eq!(handoff_exit_intent(&completion), HandoffExitIntent::None); completion }, DesktopOpenCompletion::Pending, @@ -186,11 +205,10 @@ mod tests { ); std::thread::yield_now(); } - DesktopOpenCompletion::Opened(completed) => { + DesktopOpenCompletion::HandedOff(completed) => { assert_eq!(completed, request); - let mut should_exit = false; - apply_exit_policy(&DesktopOpenCompletion::Opened(completed), &mut should_exit); - assert!(should_exit); + let intent = handoff_exit_intent(&DesktopOpenCompletion::HandedOff(completed)); + assert_eq!(intent, HandoffExitIntent::ExitExplicitly); break; } DesktopOpenCompletion::Failed { reason, .. } => { @@ -204,16 +222,24 @@ mod tests { fn failed_helper_completion_never_requests_exit() { let request = DesktopOpenRequest::ConfigFile("/tmp/config.toml".into()); let completion = classify_result(request.clone(), Err("injected failure".to_string())); - let mut should_exit = false; - apply_exit_policy(&completion, &mut should_exit); assert!(matches!( - completion, + &completion, DesktopOpenCompletion::Failed { request: failed, reason, - } if failed == request && reason == "injected failure" + } if *failed == request && reason == "injected failure" )); - assert!(!should_exit); + assert_eq!(handoff_exit_intent(&completion), HandoffExitIntent::None); + } + + #[test] + fn successful_handoff_requests_explicit_overlay_exit() { + assert_eq!( + handoff_exit_intent(&DesktopOpenCompletion::HandedOff( + DesktopOpenRequest::CaptureFolder("/tmp/capture".into()), + )), + HandoffExitIntent::ExitExplicitly + ); } } diff --git a/src/capture/clipboard.rs b/src/capture/clipboard.rs index 0f4ab698b..3702f8f13 100644 --- a/src/capture/clipboard.rs +++ b/src/capture/clipboard.rs @@ -41,7 +41,6 @@ fn copy_via_command(image_data: &[u8]) -> Result<(), CaptureError> { [OsStr::new("--type"), OsStr::new("image/png")], image_data.to_vec(), Duration::from_secs(10), - 0, ) }) .map_err(|e| { diff --git a/src/capture/sources/hyprland.rs b/src/capture/sources/hyprland.rs index f8af2b4da..7b39f5be0 100644 --- a/src/capture/sources/hyprland.rs +++ b/src/capture/sources/hyprland.rs @@ -8,10 +8,6 @@ use std::time::Duration; // Large, noisy multi-monitor PNGs can exceed the former 16 MiB transport cap. // Keep capture bounded while allowing several uncompressed 8K-sized frames. const CAPTURE_OUTPUT_CAP: usize = 256 * 1024 * 1024; -const GRIM_FULL_SCREEN_ARGUMENTS: [&str; 1] = ["-"]; -const HYPRCTL_ACTIVE_WINDOW_ARGUMENTS: [&str; 2] = ["activewindow", "-j"]; -const HYPRCTL_MONITORS_ARGUMENTS: [&str; 2] = ["monitors", "-j"]; -const SLURP_SELECTION_ARGUMENTS: [&str; 2] = ["-f", "%x,%y %wx%h"]; fn grim_geometry_arguments(geometry: &str) -> [&str; 3] { ["-g", geometry, "-"] @@ -45,7 +41,7 @@ pub async fn capture_full_screen_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Grim, "grim", - &GRIM_FULL_SCREEN_ARGUMENTS, + &["-"], Duration::from_secs(30), CAPTURE_OUTPUT_CAP, )?; @@ -81,7 +77,7 @@ pub async fn capture_active_window_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Hyprctl, "hyprctl", - &HYPRCTL_ACTIVE_WINDOW_ARGUMENTS, + &["activewindow", "-j"], Duration::from_secs(5), 2 * 1024 * 1024, )?; @@ -189,7 +185,7 @@ pub async fn capture_selection_hyprland() -> Result, CaptureError> { let output = run_helper( HelperKind::Slurp, "slurp", - &SLURP_SELECTION_ARGUMENTS, + &["-f", "%x,%y %wx%h"], Duration::from_secs(120), 4096, )?; @@ -261,7 +257,7 @@ fn hyprland_monitor_scale( let output = run_helper( HelperKind::Hyprctl, "hyprctl", - &HYPRCTL_MONITORS_ARGUMENTS, + &["monitors", "-j"], Duration::from_secs(5), 2 * 1024 * 1024, )?; @@ -317,11 +313,7 @@ mod tests { use super::*; #[test] - fn capture_helper_argv_contracts_are_explicit() { - assert_eq!(GRIM_FULL_SCREEN_ARGUMENTS, ["-"]); - assert_eq!(HYPRCTL_ACTIVE_WINDOW_ARGUMENTS, ["activewindow", "-j"]); - assert_eq!(HYPRCTL_MONITORS_ARGUMENTS, ["monitors", "-j"]); - assert_eq!(SLURP_SELECTION_ARGUMENTS, ["-f", "%x,%y %wx%h"]); + fn grim_geometry_arguments_are_explicit() { assert_eq!( grim_geometry_arguments("12,34 800x600"), ["-g", "12,34 800x600", "-"] diff --git a/src/clipboard_text.rs b/src/clipboard_text.rs index a1abcf7ef..28c7a74f0 100644 --- a/src/clipboard_text.rs +++ b/src/clipboard_text.rs @@ -18,10 +18,9 @@ pub(crate) fn copy_text_via_command(text: &str) -> Result<(), String> { broker.publish( crate::process_broker::HelperKind::WlCopy, OsStr::new("wl-copy"), - clipboard_text_copy_args(), + [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8")], text.as_bytes().to_vec(), Duration::from_secs(5), - 0, ) }) .map_err(|error| format!("Failed to run wl-copy: {error:#}"))?; @@ -39,10 +38,6 @@ pub(crate) fn copy_text_via_command(text: &str) -> Result<(), String> { Ok(()) } -fn clipboard_text_copy_args() -> [&'static OsStr; 2] { - [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8")] -} - pub(crate) fn read_clipboard_text_via_command() -> Result { let output = crate::process_broker::current() .and_then(|broker| { @@ -101,12 +96,4 @@ mod tests { ] ); } - - #[test] - fn clipboard_text_copy_publishes_one_explicit_utf8_mime() { - assert_eq!( - clipboard_text_copy_args(), - [OsStr::new("--type"), OsStr::new("text/plain;charset=utf-8"),] - ); - } } diff --git a/src/daemon/tray/helpers.rs b/src/daemon/tray/helpers.rs index 00e3a3ce4..d711a09fb 100644 --- a/src/daemon/tray/helpers.rs +++ b/src/daemon/tray/helpers.rs @@ -123,7 +123,7 @@ impl WayscriberTray { /// Open the update instructions the watcher recorded. The URL comes from /// `update_check`, which only ever hands out validated wayscriber.com links. pub(super) fn open_update_instructions(&self, url: &str) { - let invocation = match crate::desktop_open::trusted_wayscriber_url(url) { + let invocation = match crate::desktop_open::trusted_url(url) { Ok(invocation) => invocation, Err(err) => { warn!("Refused to open update instructions {url:?}: {err:#}"); @@ -131,7 +131,7 @@ impl WayscriberTray { } }; match crate::desktop_open::open_in_background(invocation) { - Ok(()) => info!("Opening update instructions"), + Ok(_worker) => info!("Opening update instructions"), Err(err) => warn!("Failed to open update instructions {url}: {err}"), } } @@ -187,7 +187,7 @@ impl WayscriberTray { let invocation = crate::desktop_open::path(&dir); match crate::desktop_open::open_in_background(invocation) { - Ok(()) => info!("Opening log directory via desktop integration"), + Ok(_worker) => info!("Opening log directory via desktop integration"), Err(err) => warn!("Failed to open log directory {}: {}", dir.display(), err), } } @@ -203,7 +203,7 @@ impl WayscriberTray { let invocation = crate::desktop_open::path(&path); match crate::desktop_open::open_in_background(invocation) { - Ok(()) => { + Ok(_worker) => { info!("Opening config file at {}", path.display()); true } diff --git a/src/desktop_open.rs b/src/desktop_open.rs index cf63470d4..8cae591dd 100644 --- a/src/desktop_open.rs +++ b/src/desktop_open.rs @@ -3,15 +3,17 @@ //! The process broker authorizes the executable and cheap argument shape. This //! module owns caller policy: paths stay paths, and outbound URLs must use the //! trusted Wayscriber HTTPS host rule before they reach the broker. +//! +//! Desktop openers are spawned with [`HelperLifetime::DetachedAfterExec`]: an +//! opener's job is to leave a descendant running, so a bounded `run` that +//! SIGKILLs the process group would kill the application it just launched. use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; -use std::time::Duration; use anyhow::{Context, Result, bail}; -const HELPER_TIMEOUT: Duration = Duration::from_secs(10); -const OUTPUT_CAP: usize = 16 * 1024; +use crate::process_broker::{HelperKind, HelperLifetime}; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct DesktopOpenInvocation { @@ -61,73 +63,49 @@ impl DesktopOpenInvocation { } } -/// Complete a bounded desktop-open operation before the caller continues. -/// Runtime owners that exit after the action use this form so broker teardown -/// cannot cancel the helper they just requested. +/// Spawn a desktop opener and return once the broker has transferred ownership. +/// Runtime owners that exit after the action use this form so teardown cannot +/// cancel a helper that must outlive the overlay. pub(crate) fn open(invocation: &DesktopOpenInvocation) -> Result<()> { let broker = crate::process_broker::current()?; - run_with_broker(&broker, invocation) + spawn_with_broker(&broker, invocation) } -/// Run a desktop opener without blocking the Wayland or tray callback that -/// requested it. The broker still owns the complete helper lifetime and -/// enforces the timeout/output policy inside the worker. -pub(crate) fn open_in_background(invocation: DesktopOpenInvocation) -> Result<()> { +/// Spawn a desktop opener without blocking the Wayland or tray callback that +/// requested it. The broker still authorizes the argv; DetachedAfterExec keeps +/// the launched application alive after this process exits. +/// +/// Callers that own the process broker must join the returned worker before +/// tearing the broker down, or a still-in-flight spawn exchange can be cancelled. +pub(crate) fn open_in_background( + invocation: DesktopOpenInvocation, +) -> Result> { let broker = crate::process_broker::current()?; std::thread::Builder::new() .name("wayscriber-desktop-open".to_string()) .spawn(move || { - if let Err(err) = run_with_broker(&broker, &invocation) { + if let Err(err) = spawn_with_broker(&broker, &invocation) { // Do not include the target or captured output: an About report // URL can carry diagnostics in its fragment. log::warn!("Desktop opener failed: {err:#}"); } }) - .context("failed to start desktop-open worker")?; - Ok(()) + .context("failed to start desktop-open worker") } -fn run_with_broker( +fn spawn_with_broker( broker: &crate::process_broker::ProcessBroker, invocation: &DesktopOpenInvocation, ) -> Result<()> { - run_with(invocation, |program, arguments, timeout, output_cap| { - broker.run( - crate::process_broker::HelperKind::DesktopOpen, - program, - arguments, + broker + .spawn( + HelperKind::DesktopOpen, + HelperLifetime::DetachedAfterExec, + invocation.program(), + invocation.arguments(), Vec::new(), - timeout, - output_cap, ) - }) -} - -fn run_with( - invocation: &DesktopOpenInvocation, - run: impl FnOnce( - &OsStr, - &[OsString], - Duration, - usize, - ) -> Result, -) -> Result<()> { - let output = run( - invocation.program(), - invocation.arguments(), - HELPER_TIMEOUT, - OUTPUT_CAP, - )?; - if output.timed_out { - bail!("desktop opener timed out"); - } - if output.status != 0 { - bail!( - "desktop opener exited unsuccessfully with status {}", - output.status - ); - } - Ok(()) + .map(|_| ()) } /// Open a local path with the platform's desktop integration. @@ -135,27 +113,15 @@ pub(crate) fn path(path: &Path) -> DesktopOpenInvocation { invocation(path.as_os_str()) } -/// Open an HTTPS URL on the update check's exact Wayscriber host allowlist. -#[cfg(any(feature = "tray", test))] -pub(crate) fn trusted_wayscriber_url(url: &str) -> Result { +/// Open an HTTPS URL on the update-check Wayscriber host allowlist +/// (`wayscriber.com` and `www.wayscriber.com`). +pub(crate) fn trusted_url(url: &str) -> Result { if !crate::update_check::is_trusted_url(url) { bail!("refusing to open an untrusted Wayscriber URL: {url:?}"); } Ok(invocation(OsStr::new(url))) } -/// About content is stricter than update metadata: every compiled-in link uses -/// the primary `https://wayscriber.com` origin. -pub(crate) fn about_url(url: &str) -> Result { - let Some(tail) = url.strip_prefix("https://wayscriber.com") else { - bail!("refusing to open an untrusted About URL: {url:?}"); - }; - if !(tail.is_empty() || tail.starts_with(['/', '?', '#'])) { - bail!("refusing to open an untrusted About URL: {url:?}"); - } - Ok(invocation(OsStr::new(url))) -} - fn invocation(target: &OsStr) -> DesktopOpenInvocation { // `cmd /C start` is intentionally absent: desktop opening must never route // through a shell. Wayscriber is a Wayland application, while `open` @@ -190,68 +156,26 @@ mod tests { } #[test] - fn desktop_open_run_declares_argv_timeout_and_output_cap() { - let invocation = path(Path::new("/tmp/Wayscriber Captures")); - let mut observed = None; - - run_with(&invocation, |program, arguments, timeout, output_cap| { - observed = Some((program.to_owned(), arguments.to_vec(), timeout, output_cap)); - Ok(crate::process_broker::BrokerOutput { - status: 0, - stdout: Vec::new(), - stderr: Vec::new(), - timed_out: false, - stdout_limit_reached: false, - }) - }) - .unwrap(); - - let (program, arguments, timeout, output_cap) = observed.unwrap(); - assert!(!matches!(program.to_str(), Some("sh" | "bash" | "cmd"))); - assert_eq!(arguments, [OsString::from("/tmp/Wayscriber Captures")]); - assert_eq!(timeout, Duration::from_secs(10)); - assert_eq!(output_cap, 16 * 1024); - } - - #[test] - fn desktop_open_run_surfaces_timeout_and_nonzero_exit() { - let invocation = path(Path::new("/tmp/capture")); - let output = |status, timed_out| crate::process_broker::BrokerOutput { - status, - stdout: Vec::new(), - stderr: Vec::new(), - timed_out, - stdout_limit_reached: false, - }; - - let timeout = run_with(&invocation, |_, _, _, _| Ok(output(137, true))).unwrap_err(); - assert!(timeout.to_string().contains("timed out")); - - let nonzero = run_with(&invocation, |_, _, _, _| Ok(output(4, false))).unwrap_err(); - assert!(nonzero.to_string().contains("status 4")); - } - - #[test] - fn about_urls_require_https_and_an_exact_wayscriber_host() { - let invocation = about_url("https://wayscriber.com/report#d=abc").unwrap(); + fn trusted_urls_share_the_update_check_host_allowlist() { + let invocation = trusted_url("https://wayscriber.com/report#d=abc").unwrap(); assert_eq!( invocation.arguments(), [OsString::from("https://wayscriber.com/report#d=abc")] ); + assert!( + trusted_url("https://www.wayscriber.com/docs/getting-started/updating.html").is_ok() + ); for untrusted in [ "http://wayscriber.com/report", "https://wayscriber.com.example/report", - "https://www.wayscriber.com/report", "https://example.com/report", "file:///etc/passwd", ] { assert!( - about_url(untrusted).is_err(), + trusted_url(untrusted).is_err(), "unexpectedly accepted {untrusted}" ); } - - assert!(trusted_wayscriber_url("https://www.wayscriber.com/docs/").is_ok()); } } diff --git a/src/input/state/core/base/state/init.rs b/src/input/state/core/base/state/init.rs index 806daf2e6..b85442241 100644 --- a/src/input/state/core/base/state/init.rs +++ b/src/input/state/core/base/state/init.rs @@ -115,6 +115,7 @@ impl InputState { active_drag_color: None, state: DrawingState::Idle, should_exit: false, + explicit_exit_requested: false, needs_redraw: true, session_dirty: false, session_preflight_options: None, diff --git a/src/input/state/core/base/state/structs.rs b/src/input/state/core/base/state/structs.rs index 20d143708..eabd37da7 100644 --- a/src/input/state/core/base/state/structs.rs +++ b/src/input/state/core/base/state/structs.rs @@ -154,6 +154,9 @@ pub struct InputState { pub state: DrawingState, /// Whether user requested to exit the overlay pub should_exit: bool, + /// Exit that must not be deferred by XDG stay-mode focus loss (for example + /// exit-after-capture). Consumed together with the Wayland explicit-close bit. + pub(crate) explicit_exit_requested: bool, /// Whether the display needs to be redrawn pub needs_redraw: bool, /// Whether session persistence should capture changes (cleared after autosave check) diff --git a/src/input/state/core/utility/toasts.rs b/src/input/state/core/utility/toasts.rs index d68e63c3c..d9ceb2e95 100644 --- a/src/input/state/core/utility/toasts.rs +++ b/src/input/state/core/utility/toasts.rs @@ -190,6 +190,19 @@ impl InputState { Some((elapsed / total).min(1.0)) } + /// Request overlay exit that must not be deferred by XDG stay-mode focus loss. + pub(crate) fn request_explicit_exit(&mut self) { + self.explicit_exit_requested = true; + self.should_exit = true; + } + + /// Take and clear the explicit-exit bit set by [`Self::request_explicit_exit`]. + pub(crate) fn take_explicit_exit_requested(&mut self) -> bool { + let was_requested = self.explicit_exit_requested; + self.explicit_exit_requested = false; + was_requested + } + /// Store image data for clipboard fallback (when clipboard copy fails). /// Used by wayland backend when capture clipboard copy fails. #[allow(dead_code)] @@ -250,7 +263,7 @@ impl InputState { } // Exit if exit-after-capture was originally enabled if fallback.exit_after_save { - self.should_exit = true; + self.request_explicit_exit(); } } Err(err) => { @@ -776,6 +789,28 @@ mod tests { assert!(state.blocked_action_feedback.is_some()); } + #[test] + fn clipboard_fallback_exit_after_save_requests_explicit_overlay_exit() { + let mut state = make_state(); + let temp = crate::test_temp::tempdir().expect("tempdir"); + state.set_clipboard_fallback( + b"not-a-real-png-but-save-writes-bytes".to_vec(), + FileSaveConfig { + save_directory: temp.path().to_path_buf(), + filename_template: "fallback".to_string(), + format: "png".to_string(), + }, + ImageOperationKind::Screenshot, + true, + ); + + state.save_pending_clipboard_to_file(); + + assert!(state.should_exit); + assert!(state.take_explicit_exit_requested()); + assert!(!state.take_explicit_exit_requested()); + } + #[test] fn canvas_clipboard_fallback_retry_failure_uses_canvas_wording() { let mut state = make_state(); diff --git a/src/process_broker/client.rs b/src/process_broker/client.rs index 48cb404c9..174b8347e 100644 --- a/src/process_broker/client.rs +++ b/src/process_broker/client.rs @@ -372,7 +372,6 @@ impl ProcessBroker { arguments: I, input: Vec, timeout: Duration, - output_cap: usize, ) -> Result where I: IntoIterator, @@ -397,7 +396,6 @@ impl ProcessBroker { environment: Vec::new(), input, timeout_ms: u64::try_from(timeout.as_millis()).unwrap_or(u64::MAX), - output_cap: output_cap.min(MAX_OUTPUT_BYTES), }, &request_descriptors, ExchangeWait::Queue, diff --git a/src/process_broker/manifest.rs b/src/process_broker/manifest.rs index 373a35e0e..cc28711a2 100644 --- a/src/process_broker/manifest.rs +++ b/src/process_broker/manifest.rs @@ -112,6 +112,24 @@ fn validate_arguments(kind: HelperKind, basename: &str, arguments: &[OsWire]) -> if arguments.first().map(|argument| argument.0.as_slice()) != Some(required_first) { bail!("update fetcher must disable user configuration in argument one"); } + // `--disable` / `--no-config` only suppress the default rc files. + // A later `--config` / `-K` would re-open that hole. + let rest = arguments.get(1..).unwrap_or(&[]); + if basename == "curl" { + if rest + .iter() + .any(|argument| is_curl_config_argument(&argument.0)) + { + bail!("update fetcher must not re-enable curl configuration after --disable"); + } + } else if rest + .iter() + .any(|argument| is_wget_config_argument(&argument.0)) + { + bail!( + "update fetcher must not re-enable wget configuration or execute directives after --no-config" + ); + } } HelperKind::DesktopOpen => { let [target] = arguments else { @@ -120,28 +138,89 @@ fn validate_arguments(kind: HelperKind, basename: &str, arguments: &[OsWire]) -> if target.0.starts_with(b"-") { bail!("desktop opener target must not be an option"); } - if let Ok(target) = std::str::from_utf8(&target.0) - && looks_like_uri(target) - && !crate::update_check::is_trusted_url(target) - { - bail!("desktop opener URL is not a trusted Wayscriber HTTPS URL"); + match std::str::from_utf8(&target.0) { + Ok(target) + if looks_like_uri(target) && !crate::update_check::is_trusted_url(target) => + { + bail!("desktop opener URL is not a trusted Wayscriber HTTPS URL"); + } + // xdg-open parses schemes bytewise. Undecodable targets that + // still look like URIs must not skip the trusted-host gate. + Err(_) if looks_like_uri_bytes(&target.0) => { + bail!("desktop opener URL must be valid UTF-8"); + } + _ => {} } } - HelperKind::Systemctl - if arguments.first().map(|argument| argument.0.as_slice()) != Some(b"--user") => - { - bail!("systemctl helper is restricted to the user service manager"); + HelperKind::Systemctl => { + if arguments.first().map(|argument| argument.0.as_slice()) != Some(b"--user") { + bail!("systemctl helper is restricted to the user service manager"); + } + // `--user` is not sticky against a later `--system` / `--global`, + // and `--machine` / `-M` can reach the system-scope bus. + if arguments + .iter() + .skip(1) + .any(|argument| is_systemctl_non_user_manager_flag(&argument.0)) + { + bail!("systemctl helper must not target the system, global, or machine manager"); + } } _ => {} } Ok(()) } +fn is_curl_config_argument(argument: &[u8]) -> bool { + if long_option_matches(argument, b"config") { + return true; + } + // Short options and clusters: `-K`, `-Kfile`, `-sK/path`, `-vK`, … + argument.starts_with(b"-") && !argument.starts_with(b"--") && argument[1..].contains(&b'K') +} + +fn is_wget_config_argument(argument: &[u8]) -> bool { + // `--no-config` skips rc files, but `--execute` / `-e` still run .wgetrc + // directives (headers, output, etc.) from the command line. + if long_option_matches(argument, b"config") || long_option_matches(argument, b"execute") { + return true; + } + // Short options and clusters: `-e`, `-ecommand`, `-qe…`, … + argument.starts_with(b"-") && !argument.starts_with(b"--") && argument[1..].contains(&b'e') +} + +fn is_systemctl_non_user_manager_flag(argument: &[u8]) -> bool { + // systemd accepts unique prefixes (`--syst`, `--glob`, `--mach`), not only + // full forms. `-M` / short clusters with `M` select a machine bus. + if long_option_matches(argument, b"system") + || long_option_matches(argument, b"global") + || long_option_matches(argument, b"machine") + { + return true; + } + argument.starts_with(b"-") && !argument.starts_with(b"--") && argument[1..].contains(&b'M') +} + +/// True when `argument` is `--name`, `--name=…`, or a non-empty unique prefix of +/// `--name` (the form getopt-style parsers accept). +fn long_option_matches(argument: &[u8], name: &[u8]) -> bool { + let Some(rest) = argument.strip_prefix(b"--") else { + return false; + }; + let option = rest.split(|&byte| byte == b'=').next().unwrap_or(rest); + !option.is_empty() && name.starts_with(option) +} + fn looks_like_uri(value: &str) -> bool { - let Some((scheme, _)) = value.split_once(':') else { + looks_like_uri_bytes(value.as_bytes()) +} + +fn looks_like_uri_bytes(value: &[u8]) -> bool { + let Some(colon) = value.iter().position(|&byte| byte == b':') else { return false; }; - let mut bytes = scheme.bytes(); + let scheme = &value[..colon]; + let mut bytes = scheme.iter().copied(); bytes.next().is_some_and(|byte| byte.is_ascii_alphabetic()) && bytes.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'+' | b'-' | b'.')) } diff --git a/src/process_broker/server.rs b/src/process_broker/server.rs index 54759cdcb..3182024d6 100644 --- a/src/process_broker/server.rs +++ b/src/process_broker/server.rs @@ -293,7 +293,6 @@ fn handle_operation( environment, input, timeout_ms, - output_cap, } => { if !supports_retained_publication(kind) { bail!("retained publication is restricted to wl-copy"); @@ -301,9 +300,8 @@ fn handle_operation( let input = decode_blob(input, descriptors, super::manifest::input_cap(kind))?; reject_descriptors(descriptors)?; super::manifest::validate(kind, &program, &arguments, &environment, &input)?; - if output_cap != 0 { - bail!("retained publication output cap must be zero"); - } + // Retained publication discards stdout/stderr structurally + // (publish_bounded uses Stdio::null); there is no output cap to enforce. let output = publish_bounded( super::manifest::command(program, arguments, environment), input, diff --git a/src/process_broker/tests.rs b/src/process_broker/tests.rs index ca4eee70d..24686e8dc 100644 --- a/src/process_broker/tests.rs +++ b/src/process_broker/tests.rs @@ -111,6 +111,20 @@ fn update_fetcher_manifest_allows_only_curl_and_wget() { ("curl", &["--silent", "--disable"][..]), ("wget", &[][..]), ("wget", &["--quiet", "--no-config"][..]), + ("curl", &["--disable", "--config", "/tmp/curlrc"][..]), + ("curl", &["--disable", "--config=/tmp/curlrc"][..]), + ("curl", &["--disable", "--conf"][..]), + ("curl", &["--disable", "-K", "/tmp/curlrc"][..]), + ("curl", &["--disable", "-K/tmp/curlrc"][..]), + ("curl", &["--disable", "-sK/path"][..]), + ("wget", &["--no-config", "--config=/tmp/wgetrc"][..]), + ("wget", &["--no-config", "--conf"][..]), + ("wget", &["--no-config", "--execute=header=X:1"][..]), + ("wget", &["--no-config", "--execute", "header=X:1"][..]), + ("wget", &["--no-config", "--exec=header=X:1"][..]), + ("wget", &["--no-config", "-e", "header=X:1"][..]), + ("wget", &["--no-config", "-eheader=X:1"][..]), + ("wget", &["--no-config", "-qeheader=X:1"][..]), ] { let program = super::wire::OsWire::from_os(OsStr::new(program)).unwrap(); assert!( @@ -190,6 +204,21 @@ fn desktop_open_manifest_accepts_one_path_or_trusted_url_without_a_shell() { ) .is_err() ); + + // Undecodable URI-shaped targets must not skip the trusted-host gate. + let mut evil = b"https://evil.example/".to_vec(); + evil.push(0x80); + let opener = super::wire::OsWire::from_os(OsStr::new("xdg-open")).unwrap(); + assert!( + super::manifest::validate( + HelperKind::DesktopOpen, + &opener, + &[super::wire::OsWire(evil)], + &[], + &[], + ) + .is_err() + ); } #[test] @@ -213,6 +242,29 @@ fn systemctl_manifest_requires_the_user_service_manager() { ) .is_err() ); + for arguments in [ + &["--user", "--system", "daemon-reload"][..], + &["--user", "--syst", "daemon-reload"][..], + &["--user", "--global", "enable", "wayscriber.service"][..], + &["--user", "--glob", "enable", "wayscriber.service"][..], + &["--user", "--machine=.host", "daemon-reload"][..], + &["--user", "--machine", ".host", "daemon-reload"][..], + &["--user", "--mach=.host", "daemon-reload"][..], + &["--user", "-M", ".host", "daemon-reload"][..], + &["--user", "-M.host", "daemon-reload"][..], + ] { + assert!( + super::manifest::validate( + HelperKind::Systemctl, + &program, + &wire_arguments(arguments), + &[], + &[], + ) + .is_err(), + "systemctl accepted non-user manager flags {arguments:?}" + ); + } } #[test] @@ -667,8 +719,7 @@ fn normal_broker_shutdown_releases_successful_provider_descendant() { pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), - 0, + Duration::from_secs(2) ) .unwrap(); assert_eq!(output.status, 0); @@ -704,7 +755,6 @@ fn shutdown_channel_peer_loss_kills_retained_provider() { ], Vec::new(), Duration::from_secs(2), - 0, ) .unwrap(); assert_eq!(output.status, 0); @@ -762,7 +812,6 @@ fn retained_publication_replacement_disposes_the_previous_provider() { ], Vec::new(), Duration::from_secs(2), - 0, ) .unwrap(); assert_eq!(first.status, 0); @@ -782,8 +831,7 @@ fn retained_publication_replacement_disposes_the_previous_provider() { second_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), - 0, + Duration::from_secs(2) ) .unwrap(); assert_eq!(second.status, 0); @@ -845,8 +893,7 @@ fn failed_publication_replacement_preserves_the_current_provider() { current_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), - 0, + Duration::from_secs(2) ) .unwrap(); assert_eq!(current.status, 0); @@ -864,7 +911,6 @@ fn failed_publication_replacement_preserves_the_current_provider() { ], Vec::new(), Duration::from_secs(2), - 0, ) .unwrap(); assert_eq!(failed.status, 7); @@ -925,7 +971,6 @@ fn retained_publication_kills_failed_or_input_stalled_provider_groups() { ], input, Duration::from_millis(100), - 0, ); let provider_pid = std::fs::read_to_string(pid_path) .unwrap() @@ -961,28 +1006,11 @@ fn retained_publication_rejects_incomplete_input_after_successful_exit() { [OsStr::new("-c"), OsStr::new("exit 0")], vec![b'x'; 1024 * 1024], Duration::from_secs(1), - 0, ); assert!(result.is_err(), "incomplete publication input was accepted"); } -#[test] -fn retained_publication_requires_an_explicit_zero_output_cap() { - let guard = start_for_runtime().unwrap(); - let result = guard.broker().publish( - HelperKind::TestShell, - OsStr::new("sh"), - [OsStr::new("-c"), OsStr::new("exit 0")], - Vec::new(), - Duration::from_secs(1), - 1, - ); - - let error = result.expect_err("nonzero publication output cap was accepted"); - assert!(format!("{error:#}").contains("output cap must be zero")); -} - #[test] fn broker_shutdown_preempts_retained_publication_stdin_writer() { let guard = start_for_runtime().unwrap(); @@ -1008,8 +1036,7 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { current_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), - 0, + Duration::from_secs(2) ) .unwrap(); assert_eq!(current.status, 0); @@ -1032,7 +1059,6 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { ], vec![b'x'; 1024 * 1024], Duration::from_secs(2), - 0, ) } }); @@ -1096,7 +1122,6 @@ fn wl_copy_publication_accepts_capture_sized_input() { // observed to exceed 5 s when the full parallel suite saturates // the machine - the broker then SIGKILLs the helper (status 137). Duration::from_secs(30), - 0, ) .unwrap(); diff --git a/src/process_broker/wire.rs b/src/process_broker/wire.rs index 23853c3ff..ac5827606 100644 --- a/src/process_broker/wire.rs +++ b/src/process_broker/wire.rs @@ -101,8 +101,6 @@ pub(super) enum BrokerOperation { environment: Vec<(OsWire, Option)>, input: BlobWire, timeout_ms: u64, - /// Retained publication discards output, so callers must declare zero. - output_cap: usize, }, Spawn { kind: HelperKind,