diff --git a/src/about_window/clipboard.rs b/src/about_window/clipboard.rs index 7debe6b2b..dc4d633a2 100644 --- a/src/about_window/clipboard.rs +++ b/src/about_window/clipboard.rs @@ -1,41 +1,32 @@ -use anyhow::{Context, Result}; +use anyhow::Result; use log::warn; +use std::thread::JoinHandle; -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| { + crate::desktop_open::open_in_background(invocation.clone()) + }) +} - if let Err(err) = cmd.spawn() { - warn!("Failed to open URL {}: {}", url, err); - } +fn open_url_with( + url: &str, + 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 commit id to clipboard: {}", err); + warn!("Failed to copy About text to clipboard: {err:#}"); } - }); + })) } fn copy_text_with_command(text: &str, command_copy: C) -> Result<()> @@ -49,32 +40,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 +49,67 @@ mod tests { use super::*; + #[test] + fn open_url_builds_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 open 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://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 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 c004348fa..23f5138c3 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; @@ -11,8 +11,10 @@ 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 REPORTED_NOTICE: &str = "Diagnostics copied — paste them if the form asks"; +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 — opening browser"; +const REPORT_OPEN_FAILED_NOTICE: &str = "Diagnostics copied — browser open failed"; impl AboutWindowState { #[allow(clippy::too_many_arguments)] @@ -49,6 +51,7 @@ impl AboutWindowState { should_exit: false, needs_redraw: true, check_requested: false, + helper_workers: Vec::new(), content, plan, update, @@ -124,27 +127,59 @@ 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(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 } => { - clipboard::copy_text_to_clipboard(&diagnostics); - clipboard::open_url(&url); - self.set_notice(REPORTED_NOTICE); + // Start the desktop-open worker before the independent clipboard + // publication worker. + let opened = clipboard::open_url(&url); + if let Some(worker) = clipboard::copy_text_to_clipboard(&diagnostics) { + self.track_helper_worker(worker); + } + match opened { + 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); + } + } } AboutAction::CheckForUpdates => self.begin_update_check(), AboutAction::Close => self.should_exit = true, } } + 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/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/backend/wayland/backend/event_loop/capture.rs b/src/backend/wayland/backend/event_loop/capture.rs index a35fb4383..4ea35ba70 100644 --- a/src/backend/wayland/backend/event_loop/capture.rs +++ b/src/backend/wayland/backend/event_loop/capture.rs @@ -97,6 +97,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(); @@ -137,6 +138,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(); } @@ -411,6 +413,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 653f5b5a6..a5bb0460b 100644 --- a/src/backend/wayland/backend/event_loop/mod.rs +++ b/src/backend/wayland/backend/event_loop/mod.rs @@ -194,23 +194,13 @@ 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()) { if state.xdg_focus_loss_exits_overlay() { @@ -278,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(); @@ -405,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; @@ -436,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/mod.rs b/src/backend/wayland/clipboard/mod.rs index bac9484ec..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, -}; - 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..88254160f 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. 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 handoff" + ); + } + 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 66b23a203..0b1e2aaaf 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, @@ -73,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, @@ -101,6 +100,7 @@ mod clipboard; mod color_picker; mod core; mod data; +mod desktop_open; mod eyedropper; mod gtk_toolbar; mod helpers; @@ -250,18 +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: 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 252f08ce5..ffb1e5d5f 100644 --- a/src/backend/wayland/state/core/init.rs +++ b/src/backend/wayland/state/core/init.rs @@ -101,25 +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 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 { @@ -148,6 +142,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..6614ca6f0 --- /dev/null +++ b/src/backend/wayland/state/desktop_open.rs @@ -0,0 +1,245 @@ +//! Runtime-owned desktop-open completion. +//! +//! 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 been handed off successfully. + +use std::time::Duration; + +use super::{RuntimeOperationController, WaylandState}; +use crate::backend::wayland::{RuntimeOperationPoll, RuntimeOperationSubmitFailure}; +use crate::desktop_open::{DesktopOpenInvocation, DesktopOpenRequest}; +use crate::input::state::{Toast, ToastPriority}; + +enum DesktopOpenCompletion { + Pending, + /// 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) = + 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()); + 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::HandedOff(request) => { + log::info!( + "Handed off desktop open for {} 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 RuntimeOperationController>, + request: DesktopOpenRequest, + open: impl FnOnce(&DesktopOpenInvocation) -> anyhow::Result<()> + Send + 'static, +) -> Result<(), RuntimeOperationSubmitFailure> { + 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: RuntimeOperationPoll>, +) -> DesktopOpenCompletion { + match poll { + RuntimeOperationPoll::Idle | RuntimeOperationPoll::Pending { .. } => { + DesktopOpenCompletion::Pending + } + RuntimeOperationPoll::Ready { + context: request, + outcome, + .. + } => classify_result(request, outcome), + RuntimeOperationPoll::ProducerFailed { + context: request, + reason, + .. + } => DesktopOpenCompletion::Failed { request, reason }, + RuntimeOperationPoll::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::HandedOff(request), + Err(reason) => DesktopOpenCompletion::Failed { request, reason }, + } +} + +#[cfg(test)] +mod tests { + use std::sync::mpsc; + use std::time::{Duration, Instant}; + + use super::*; + 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 = + 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(); + 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!(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()); + assert_eq!(handoff_exit_intent(&completion), HandoffExitIntent::None); + 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::HandedOff(completed) => { + assert_eq!(completed, request); + let intent = handoff_exit_intent(&DesktopOpenCompletion::HandedOff(completed)); + assert_eq!(intent, HandoffExitIntent::ExitExplicitly); + 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())); + + assert!(matches!( + &completion, + DesktopOpenCompletion::Failed { + request: failed, + reason, + } if *failed == request && reason == "injected failure" + )); + 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/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(); 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/capture/sources/hyprland.rs b/src/capture/sources/hyprland.rs index a18e5b337..7b39f5be0 100644 --- a/src/capture/sources/hyprland.rs +++ b/src/capture/sources/hyprland.rs @@ -9,6 +9,10 @@ use std::time::Duration; // Keep capture bounded while allowing several uncompressed 8K-sized frames. const CAPTURE_OUTPUT_CAP: usize = 256 * 1024 * 1024; +fn grim_geometry_arguments(geometry: &str) -> [&str; 3] { + ["-g", geometry, "-"] +} + fn run_helper( kind: HelperKind, program: &str, @@ -145,10 +149,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, )?; @@ -208,10 +213,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, )?; @@ -301,3 +307,16 @@ fn hyprland_monitor_scale( Ok(None) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn grim_geometry_arguments_are_explicit() { + assert_eq!( + grim_geometry_arguments("12,34 800x600"), + ["-g", "12,34 800x600", "-"] + ); + } +} diff --git a/src/daemon/setup.rs b/src/daemon/setup.rs index 1271488ef..16a517efb 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(OsStr::new)) + .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..d711a09fb 100644 --- a/src/daemon/tray/helpers.rs +++ b/src/daemon/tray/helpers.rs @@ -38,25 +38,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,12 +123,15 @@ 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) { - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - OsStr::new("xdg-open"), - &[OsString::from(url)], - ) { - Ok(child) => info!("Opened update instructions {url} (pid {})", child.id()), + let invocation = match crate::desktop_open::trusted_url(url) { + Ok(invocation) => invocation, + Err(err) => { + warn!("Refused to open update instructions {url:?}: {err:#}"); + return; + } + }; + match crate::desktop_open::open_in_background(invocation) { + Ok(_worker) => info!("Opening update instructions"), Err(err) => warn!("Failed to open update instructions {url}: {err}"), } } @@ -201,12 +185,9 @@ impl WayscriberTray { return; } - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - OsStr::new("xdg-open"), - &[dir.as_os_str().into()], - ) { - Ok(child) => info!("Opened log directory via xdg-open (pid {})", child.id()), + let invocation = crate::desktop_open::path(&dir); + match crate::desktop_open::open_in_background(invocation) { + Ok(_worker) => info!("Opening log directory via desktop integration"), Err(err) => warn!("Failed to open log directory {}: {}", dir.display(), err), } } @@ -220,18 +201,10 @@ impl WayscriberTray { } }; - let (opener, arguments) = opener_arguments(&path); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, - ) { - Ok(child) => { - info!( - "Opened config file at {} (pid {})", - path.display(), - child.id() - ); + let invocation = crate::desktop_open::path(&path); + match crate::desktop_open::open_in_background(invocation) { + Ok(_worker) => { + info!("Opening config file at {}", path.display()); true } Err(err) => { 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"), + ] + ); + } +} diff --git a/src/desktop_open.rs b/src/desktop_open.rs new file mode 100644 index 000000000..8cae591dd --- /dev/null +++ b/src/desktop_open.rs @@ -0,0 +1,181 @@ +//! 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. +//! +//! 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 anyhow::{Context, Result, bail}; + +use crate::process_broker::{HelperKind, HelperLifetime}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DesktopOpenInvocation { + program: OsString, + 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 + } + + pub(crate) fn arguments(&self) -> &[OsString] { + &self.arguments + } +} + +/// 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()?; + spawn_with_broker(&broker, invocation) +} + +/// 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) = 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") +} + +fn spawn_with_broker( + broker: &crate::process_broker::ProcessBroker, + invocation: &DesktopOpenInvocation, +) -> Result<()> { + broker + .spawn( + HelperKind::DesktopOpen, + HelperLifetime::DetachedAfterExec, + invocation.program(), + invocation.arguments(), + Vec::new(), + ) + .map(|_| ()) +} + +/// 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 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))) +} + +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 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://example.com/report", + "file:///etc/passwd", + ] { + assert!( + trusted_url(untrusted).is_err(), + "unexpectedly accepted {untrusted}" + ); + } + } +} 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/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 3f081ed7f..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,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. /// @@ -135,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( @@ -175,36 +157,10 @@ impl InputState { return; }; - let (opener, arguments) = opener_arguments(&folder); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, - ) { - Ok(child) => { - log::info!( - "Opened capture folder at {} (pid {})", - folder.display(), - child.id() - ); - self.should_exit = true; - } - 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. @@ -222,30 +178,10 @@ impl InputState { } }; - let (opener, arguments) = opener_arguments(&path); - match spawn_detached( - crate::process_broker::HelperKind::DesktopOpen, - &opener, - &arguments, - ) { - Ok(child) => { - log::info!( - "Opened config file at {} (pid {})", - path.display(), - child.id() - ); - self.should_exit = true; - 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 - } - } + 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/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/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/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 diff --git a/src/process_broker/manifest.rs b/src/process_broker/manifest.rs index c86607be2..cc28711a2 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,132 @@ 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"); + } + // `--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 { + bail!("desktop opener requires exactly one target argument"); + }; + if target.0.starts_with(b"-") { + bail!("desktop opener target must not be an option"); + } + 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"); + } + // `--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 { + 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 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'.')) +} + pub(super) fn command( program: OsWire, arguments: Vec, 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}; diff --git a/src/process_broker/server.rs b/src/process_broker/server.rs index 442345aa7..3182024d6 100644 --- a/src/process_broker/server.rs +++ b/src/process_broker/server.rs @@ -300,6 +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)?; + // 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 a7b84256e..24686e8dc 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,17 +79,194 @@ 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"][..]), + ("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!( + 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() + ); + + // 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] +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() + ); + 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] fn tesseract_manifest_allows_only_the_tesseract_basename() { for program in ["/usr/bin/tesseract", "/usr/local/bin/tesseract"] { @@ -535,7 +719,7 @@ fn normal_broker_shutdown_releases_successful_provider_descendant() { pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), + Duration::from_secs(2) ) .unwrap(); assert_eq!(output.status, 0); @@ -647,7 +831,7 @@ fn retained_publication_replacement_disposes_the_previous_provider() { second_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), + Duration::from_secs(2) ) .unwrap(); assert_eq!(second.status, 0); @@ -709,7 +893,7 @@ fn failed_publication_replacement_preserves_the_current_provider() { current_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), + Duration::from_secs(2) ) .unwrap(); assert_eq!(current.status, 0); @@ -852,7 +1036,7 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { current_pid_path.as_os_str(), ], Vec::new(), - Duration::from_secs(2), + Duration::from_secs(2) ) .unwrap(); assert_eq!(current.status, 0); 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; 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"),