From d762f8cd3acd93932fafd1e20cbe61dcb952b4cf Mon Sep 17 00:00:00 2001 From: Lahfir Date: Wed, 10 Jun 2026 17:48:46 -0700 Subject: [PATCH] fix: enforce the protected-process guard inside the adapter close path The guard lived only in the CLI command layer, so ad_close_app could force-kill session-critical processes (loginwindow, WindowServer, Dock) that the CLI refuses. close_app_impl now refuses them before any side effect with the exact CLI error contract, making CLI, FFI, and any future consumer behave identically; the CLI preflight remains as an earlier check against the same predicate. --- crates/ffi/src/apps/close.rs | 6 +++++- crates/macos/src/system/app_ops.rs | 19 +++++++++++++++++++ crates/macos/src/system/app_ops_tests.rs | 10 ++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/crates/ffi/src/apps/close.rs b/crates/ffi/src/apps/close.rs index 89fc762..343fee4 100644 --- a/crates/ffi/src/apps/close.rs +++ b/crates/ffi/src/apps/close.rs @@ -6,7 +6,11 @@ use std::os::raw::c_char; /// Closes the application identified by `id` (bundle id on macOS, /// executable path on other platforms). `force = true` skips the -/// graceful-shutdown path (equivalent to `kill -9`). +/// graceful-shutdown path (equivalent to `kill -9`). Session-critical +/// processes (loginwindow, WindowServer, Dock, Finder, launchd) are +/// refused with `AD_RESULT_ERR_INVALID_ARGS` — the protected-process +/// guard is enforced inside the adapter, so FFI and CLI behave +/// identically. /// /// # Safety /// `adapter` must be non-null. `id` must be a non-null UTF-8 C string. diff --git a/crates/macos/src/system/app_ops.rs b/crates/macos/src/system/app_ops.rs index c9a62e3..e444d6a 100644 --- a/crates/macos/src/system/app_ops.rs +++ b/crates/macos/src/system/app_ops.rs @@ -194,12 +194,31 @@ pub fn is_protected_process(identifier: &str) -> bool { PROTECTED_PROCESSES.iter().any(|p| lower.contains(p)) } +fn ensure_not_protected(id: &str) -> Result<(), AdapterError> { + if is_protected_process(id) { + return Err(AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + format!("'{id}' is a protected system process and cannot be closed"), + ) + .with_suggestion( + "Target a regular application; session-critical processes (loginwindow, WindowServer, Dock, Finder, launchd) are never closed.", + )); + } + Ok(()) +} + #[cfg(test)] #[path = "app_ops_tests.rs"] mod tests; +/// Closes an app after the protected-process guard. The guard runs here — +/// inside the adapter — so every consumer (CLI, FFI, future MCP) refuses +/// session-critical processes identically; the CLI command's own preflight +/// is an earlier check against the same predicate, not the enforcement +/// point. The error mirrors the CLI contract exactly (code and message). #[cfg(target_os = "macos")] pub fn close_app_impl(id: &str, force: bool) -> Result<(), AdapterError> { + ensure_not_protected(id)?; tracing::debug!("system: close app={id:?} force={force}"); use std::process::Command; use std::time::Duration; diff --git a/crates/macos/src/system/app_ops_tests.rs b/crates/macos/src/system/app_ops_tests.rs index f4296b8..8c4a0cc 100644 --- a/crates/macos/src/system/app_ops_tests.rs +++ b/crates/macos/src/system/app_ops_tests.rs @@ -20,3 +20,13 @@ fn ordinary_apps_are_not_protected() { assert!(!is_protected_process("Safari")); assert!(!is_protected_process("com.company.MyApp")); } + +#[test] +fn adapter_guard_refuses_protected_processes_with_the_cli_contract() { + let err = ensure_not_protected("loginwindow").unwrap_err(); + + assert_eq!(err.code, agent_desktop_core::error::ErrorCode::InvalidArgs); + assert!(err.message.contains("protected")); + assert!(err.suggestion.is_some()); + assert!(ensure_not_protected("TextEdit").is_ok()); +}