From 822dc84435416f7257e99180c08afb65601dc3e5 Mon Sep 17 00:00:00 2001 From: Sergei Razmetov Date: Mon, 26 Oct 2026 18:34:11 +0300 Subject: [PATCH] fix(codex): restart the daemon without Wizard's TCC attribution macOS charged the Codex daemon that Wizard restarted, and everything Codex ran in it, to Wizard, so users saw "CoreInfra Wizard tried to access data from other apps" notifications. - When Wizard is its own responsible process (the app), spawn the restart with responsibility disclaimed. A CLI run from a terminal keeps the terminal's attribution. - Use `daemon bootstrap` instead of `restart`, so a running updater is replaced too. An existing updater may still be attributed to an older Wizard, and it keeps proxy variables that were removed from .env. Preserve remote control, which bootstrap would otherwise reset. - Skip the restart unless `daemon version` reports a managed daemon. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/wizard-core/Cargo.toml | 1 + crates/wizard-core/src/agents/codex.rs | 99 ++++-- crates/wizard-core/src/platform/mod.rs | 20 +- .../src/platform/responsibility.rs | 288 ++++++++++++++++++ 6 files changed, 379 insertions(+), 31 deletions(-) create mode 100644 crates/wizard-core/src/platform/responsibility.rs diff --git a/Cargo.lock b/Cargo.lock index 0e3334f..20ae512 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5404,6 +5404,7 @@ dependencies = [ "cookie", "dirs 6.0.0", "jsonc-parser", + "libc", "log", "objc2-app-kit", "objc2-foundation", diff --git a/Cargo.toml b/Cargo.toml index e80741d..30a3c15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ anyhow = "1.0.104" chrono = { version = "0.4.45", default-features = false, features = ["serde", "clock"] } dirs = "6" jsonc-parser = { version = "0.34.0", features = ["cst", "serde_json"] } +libc = "0.2.189" base64 = "0.22.1" clap = "4.5.60" cookie = { version = "0.18.2", default-features = false } diff --git a/crates/wizard-core/Cargo.toml b/crates/wizard-core/Cargo.toml index 45801a4..e67ea20 100644 --- a/crates/wizard-core/Cargo.toml +++ b/crates/wizard-core/Cargo.toml @@ -30,6 +30,7 @@ uuid.workspace = true wizard-proxy.workspace = true [target.'cfg(target_os = "macos")'.dependencies] +libc.workspace = true objc2-app-kit.workspace = true objc2-foundation.workspace = true plist.workspace = true diff --git a/crates/wizard-core/src/agents/codex.rs b/crates/wizard-core/src/agents/codex.rs index 5a14036..ce40da5 100644 --- a/crates/wizard-core/src/agents/codex.rs +++ b/crates/wizard-core/src/agents/codex.rs @@ -8,13 +8,15 @@ use crate::{ backups::{AgentKind, BackupManager}, changes::{FileChange, FileSnapshot}, }, - platform::{command_output, env_var_not_empty, find_executable, is_executable}, + platform::{ + command_output, daemon_command_status, env_var_not_empty, find_executable, is_executable, + }, prepared_plan::{PreparedAgent, PreparedPlan, PreparedStep}, settings::Settings, }; use anyhow::{Context as _, Result, ensure}; use core::time::Duration; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use std::{ path::{Path, PathBuf}, process::Stdio, @@ -105,49 +107,86 @@ pub fn restore_backup(id: u32) -> Result<()> { BackupManager::load_backup(AgentKind::Codex, id) } -/// Call from a Tokio blocking worker. Restart only if the version probe succeeds. +/// Call from a Tokio blocking worker. Restart only if the version probe finds a managed daemon. /// A restart may interrupt active work; this does not change routing configuration. pub fn restart_daemon(executable: &Path, codex_home: &Path) -> Result<()> { - let mut probe = tokio::process::Command::new(executable); - probe - .args(["app-server", "daemon", "version"]) - .env("CODEX_HOME", codex_home) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - - // TODO: Distinguish an absent daemon from command failures and unsupported versions. - // For now, spawn errors, timeouts and nonzero exit statuses all mean "no daemon". - if !matches!(command_output(probe, Duration::from_secs(15)), Ok(output) if output.status.success()) - { - log::info!("Codex daemon restart skipped: version probe did not succeed"); + if !managed_daemon_running(executable, codex_home) { + log::info!("Codex daemon restart skipped: no managed daemon is running"); return Ok(()); } - log::info!("Restarting Codex daemon"); + // Unlike `daemon restart`, bootstrap also replaces a running updater, which relaunches the + // daemon on updates. It keeps variables later removed from `.env`, and one started by an + // older Wizard stays attributed to Wizard by macOS. Bootstrap resets remote control, though. let mut command = tokio::process::Command::new(executable); command - .args(["app-server", "daemon", "restart"]) - .env("CODEX_HOME", codex_home) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()); + .args(["app-server", "daemon", "bootstrap"]) + .env("CODEX_HOME", codex_home); + if remote_control_enabled(codex_home)? { + command.arg("--remote-control"); + } - // Allow the daemon's maximum five-minute graceful shutdown, plus startup time. - let output = command_output(command, Duration::from_secs(360)).inspect_err(|_| { - log::error!("Codex daemon restart failed: command could not complete"); - })?; - if !output.status.success() { - log::error!("Codex daemon restart failed with {}", output.status); + log::info!("Restarting Codex daemon"); + // Allow the daemon's maximum five-minute graceful shutdown, plus startup and updater readiness. + let status = daemon_command_status(command, Duration::from_secs(420)) + .inspect_err(|_| { + log::error!("Codex daemon restart failed: command could not complete"); + }) + .context( + "Codex daemon restart failed; run `codex app-server daemon restart` in a terminal", + )?; + if !status.success() { + log::error!("Codex daemon restart failed with {status}"); } ensure!( - output.status.success(), - "Codex daemon restart failed with {}; run the command manually for diagnostics", - output.status + status.success(), + "Codex daemon restart failed with {status}; run `codex app-server daemon restart` in a terminal" ); log::info!("Codex daemon restarted successfully"); Ok(()) } +/// Whether `daemon version` reports an app-server that the daemon manages. +fn managed_daemon_running(executable: &Path, codex_home: &Path) -> bool { + #[derive(Deserialize)] + struct Version { + backend: Option, + } + + let mut probe = tokio::process::Command::new(executable); + probe + .args(["app-server", "daemon", "version"]) + .env("CODEX_HOME", codex_home) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + // TODO: Distinguish an absent daemon from command failures and unsupported versions. + // For now, spawn errors, timeouts, nonzero exit statuses and unmanaged servers all mean + // "no daemon". + command_output(probe, Duration::from_secs(15)) + .ok() + .filter(|output| output.status.success()) + .and_then(|output| serde_json::from_slice::(&output.stdout).ok()) + .is_some_and(|version| version.backend.as_deref() == Some("pid")) +} + +fn remote_control_enabled(codex_home: &Path) -> Result { + let path = codex_home.join("app-server-daemon/settings.json"); + let content = match std::fs::read_to_string(&path) { + Ok(content) => content, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) => { + return Err(error).with_context(|| format!("failed to read {}", path.display())); + } + }; + let settings: serde_json::Value = serde_json::from_str(&content) + .with_context(|| format!("failed to parse {}", path.display()))?; + Ok(settings + .get("remoteControlEnabled") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false)) +} + /// Prepare without writing files or probing the daemon. `token` is ignored when disabled. pub fn prepare_routing( mode: RoutingMode, diff --git a/crates/wizard-core/src/platform/mod.rs b/crates/wizard-core/src/platform/mod.rs index ed5d1d4..cce6a3d 100644 --- a/crates/wizard-core/src/platform/mod.rs +++ b/crates/wizard-core/src/platform/mod.rs @@ -3,11 +3,13 @@ use core::time::Duration; use std::{ ffi::OsString, path::{Path, PathBuf}, - process::Output, + process::{ExitStatus, Output, Stdio}, }; #[cfg(target_os = "macos")] pub(crate) mod macos; +#[cfg(target_os = "macos")] +mod responsibility; /// Reads a nonempty environment variable, logging only its name when unavailable. pub(crate) fn env_var_not_empty(name: &str) -> Option { @@ -179,6 +181,22 @@ pub(crate) fn command_output( run_command(command, None, timeout) } +/// Runs a command that starts long-lived background processes, with null stdio and bounded +/// execution time. On macOS, when this process is its own responsible process (the app, not a +/// CLI in a terminal), the command becomes its own, so TCC does not attribute the processes it +/// starts to Wizard. Must be called inside `spawn_blocking`. +pub(crate) fn daemon_command_status( + mut command: tokio::process::Command, + timeout: Duration, +) -> Result { + #[cfg(target_os = "macos")] + if responsibility::is_self_responsible() { + return responsibility::command_status_disclaimed(command.as_std(), timeout); + } + command.stdout(Stdio::null()).stderr(Stdio::null()); + Ok(command_output(command, timeout)?.status) +} + /// Like `command_output`, but writes `input` to the command's stdin and closes it. /// Use this to pass secrets instead of command arguments. pub(crate) fn command_output_with_stdin( diff --git a/crates/wizard-core/src/platform/responsibility.rs b/crates/wizard-core/src/platform/responsibility.rs new file mode 100644 index 0000000..435f993 --- /dev/null +++ b/crates/wizard-core/src/platform/responsibility.rs @@ -0,0 +1,288 @@ +//! macOS charges privacy (TCC) access to a process's "responsible process", which children +//! inherit through fork, exec, `setsid` and reparenting to launchd. Daemons started by the app +//! would otherwise be attributed to Wizard for their whole lifetime. +//! The responsibility functions are private libSystem API (also used by LLDB and Chromium), +//! so they are resolved at runtime instead of linked. +#![expect( + unsafe_code, + reason = "posix_spawn and the private responsibility functions have no safe bindings" +)] + +extern crate alloc; + +use alloc::{collections::BTreeMap, ffi::CString}; +use anyhow::{Context as _, Result, bail}; +use core::{ + ffi::{CStr, c_char, c_int, c_short, c_void}, + time::Duration, +}; +use std::{ + ffi::OsString, + os::unix::{ + ffi::{OsStrExt as _, OsStringExt as _}, + process::ExitStatusExt as _, + }, + process::{Command, ExitStatus}, + time::Instant, +}; + +type GetResponsiblePid = unsafe extern "C" fn(libc::pid_t) -> libc::pid_t; +type SetDisclaim = unsafe extern "C" fn(*mut libc::posix_spawnattr_t, c_int) -> c_int; + +const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(100); + +fn symbol(name: &CStr) -> Option<*mut c_void> { + // SAFETY: `name` is NUL-terminated; RTLD_DEFAULT only searches loaded images. + let symbol = unsafe { libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr()) }; + (!symbol.is_null()).then_some(symbol) +} + +/// Reports a process itself once its responsible process has exited. +fn responsible_pid(pid: libc::pid_t) -> Option { + let function = symbol(c"responsibility_get_pid_responsible_for_pid")?; + // SAFETY: matches Apple's private declaration, as used by LLDB. + let function = unsafe { core::mem::transmute::<*mut c_void, GetResponsiblePid>(function) }; + // SAFETY: the function only reads kernel process information. + Some(unsafe { function(pid) }) +} + +/// True for an app launched by launchd; false for a CLI charged to its terminal. +/// Also false when the lookup is unavailable or fails, keeping the ordinary attribution. +pub(crate) fn is_self_responsible() -> bool { + libc::pid_t::try_from(std::process::id()).is_ok_and(|pid| responsible_pid(pid) == Some(pid)) +} + +struct SpawnAttributes(libc::posix_spawnattr_t); + +impl Drop for SpawnAttributes { + fn drop(&mut self) { + // SAFETY: constructed only after a successful posix_spawnattr_init. + unsafe { libc::posix_spawnattr_destroy(&raw mut self.0) }; + } +} + +struct FileActions(libc::posix_spawn_file_actions_t); + +impl Drop for FileActions { + fn drop(&mut self) { + // SAFETY: constructed only after a successful posix_spawn_file_actions_init. + unsafe { libc::posix_spawn_file_actions_destroy(&raw mut self.0) }; + } +} + +fn check(code: c_int, operation: &str) -> Result<()> { + if code == 0 { + return Ok(()); + } + Err(std::io::Error::from_raw_os_error(code)).with_context(|| format!("{operation} failed")) +} + +fn c_string(bytes: &[u8]) -> Result { + CString::new(bytes).context("command contains a NUL byte") +} + +/// Runs `command` as its own responsible process, with null stdio, and waits for it. +/// Uses only the program, arguments and environment changes. +/// Must be called inside `spawn_blocking`. +pub(crate) fn command_status_disclaimed( + command: &Command, + timeout: Duration, +) -> Result { + let set_disclaim = symbol(c"responsibility_spawnattrs_setdisclaim") + .context("this macOS version cannot start a process as its own responsible process")?; + // SAFETY: matches Apple's private declaration, as used by LLDB. + let set_disclaim = unsafe { core::mem::transmute::<*mut c_void, SetDisclaim>(set_disclaim) }; + + let program_name = command.get_program().to_string_lossy().into_owned(); + let program = c_string(command.get_program().as_bytes())?; + let mut arguments = vec![program.clone()]; + for argument in command.get_args() { + arguments.push(c_string(argument.as_bytes())?); + } + let mut env: BTreeMap = std::env::vars_os().collect(); + for (key, value) in command.get_envs() { + match value { + Some(value) => env.insert(key.to_owned(), value.to_owned()), + None => env.remove(key), + }; + } + let env = env + .into_iter() + .map(|(key, value)| { + let mut entry = key.into_vec(); + entry.push(b'='); + entry.extend_from_slice(value.as_bytes()); + c_string(&entry) + }) + .collect::>>()?; + let argv: Vec<*mut c_char> = arguments + .iter() + .map(|argument| argument.as_ptr().cast_mut()) + .chain([core::ptr::null_mut()]) + .collect(); + let envp: Vec<*mut c_char> = env + .iter() + .map(|entry| entry.as_ptr().cast_mut()) + .chain([core::ptr::null_mut()]) + .collect(); + + // Match std::process::Command: reset the signal mask and libstd's ignored SIGPIPE. + // Unlike it, close every inherited descriptor, such as the app's proxy listeners. + let flags = c_short::try_from( + libc::POSIX_SPAWN_CLOEXEC_DEFAULT + | libc::POSIX_SPAWN_SETSIGMASK + | libc::POSIX_SPAWN_SETSIGDEF, + ) + .context("invalid posix_spawn flags")?; + let mut pid: libc::pid_t = 0; + // SAFETY: each object is initialized before use and destroyed by its guard; all pointers + // reference NUL-terminated strings and NULL-terminated arrays that outlive posix_spawn. + unsafe { + let mut attributes = core::ptr::null_mut(); + check( + libc::posix_spawnattr_init(&raw mut attributes), + "posix_spawnattr_init", + )?; + let mut attributes = SpawnAttributes(attributes); + let mut actions = core::ptr::null_mut(); + check( + libc::posix_spawn_file_actions_init(&raw mut actions), + "posix_spawn_file_actions_init", + )?; + let mut actions = FileActions(actions); + + check( + libc::posix_spawnattr_setflags(&raw mut attributes.0, flags), + "posix_spawnattr_setflags", + )?; + let mut signals: libc::sigset_t = 0; + libc::sigemptyset(&raw mut signals); + check( + libc::posix_spawnattr_setsigmask(&raw mut attributes.0, &raw const signals), + "posix_spawnattr_setsigmask", + )?; + libc::sigaddset(&raw mut signals, libc::SIGPIPE); + check( + libc::posix_spawnattr_setsigdefault(&raw mut attributes.0, &raw const signals), + "posix_spawnattr_setsigdefault", + )?; + check( + set_disclaim(&raw mut attributes.0, 1), + "responsibility_spawnattrs_setdisclaim", + )?; + for (fd, flags) in [ + (libc::STDIN_FILENO, libc::O_RDONLY), + (libc::STDOUT_FILENO, libc::O_WRONLY), + (libc::STDERR_FILENO, libc::O_WRONLY), + ] { + check( + libc::posix_spawn_file_actions_addopen( + &raw mut actions.0, + fd, + c"/dev/null".as_ptr(), + flags, + 0, + ), + "posix_spawn_file_actions_addopen", + )?; + } + + log::debug!("running command as its own responsible process: {program_name}"); + check( + libc::posix_spawn( + &raw mut pid, + program.as_ptr(), + &raw const actions.0, + &raw const attributes.0, + argv.as_ptr(), + envp.as_ptr(), + ), + "posix_spawn", + ) + .with_context(|| format!("failed to run {program_name}"))?; + } + + let deadline = Instant::now() + timeout; + loop { + let mut status = 0; + // SAFETY: `pid` is our unreaped child; Tokio and std reap only children they spawned. + match unsafe { libc::waitpid(pid, &raw mut status, libc::WNOHANG) } { + 0 => {} + -1 => { + let error = std::io::Error::last_os_error(); + if error.kind() != std::io::ErrorKind::Interrupted { + return Err(error).context("failed to wait for command"); + } + } + _ => return Ok(ExitStatus::from_raw(status)), + } + if Instant::now() >= deadline { + // SAFETY: as above; the blocking wait reaps the killed child. + unsafe { + libc::kill(pid, libc::SIGKILL); + libc::waitpid(pid, &raw mut status, 0); + } + bail!( + "{program_name} timed out after {} seconds; the operation may be partially completed", + timeout.as_secs() + ); + } + std::thread::sleep(WAIT_POLL_INTERVAL); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn disclaimed_command_reports_exit_status() { + let mut command = Command::new("/bin/sh"); + command.args(["-c", "test \"$WIZARD_TEST\" = set && exit 3"]); + command.env("WIZARD_TEST", "set"); + + let status = command_status_disclaimed(&command, Duration::from_secs(10)) + .expect("disclaimed command runs"); + + assert_eq!(status.code(), Some(3)); + } + + #[test] + fn disclaimed_command_times_out() { + let mut command = Command::new("/bin/sleep"); + command.arg("10"); + + let error = command_status_disclaimed(&command, Duration::from_millis(200)) + .expect_err("command exceeds its timeout"); + + assert!(format!("{error:#}").contains("timed out")); + } + + #[test] + fn detached_descendant_is_not_charged_to_this_process() { + let directory = tempfile::tempdir().expect("temporary directory"); + let pid_file = directory.path().join("pid"); + let mut command = Command::new("/bin/sh"); + command + .args(["-c", "/bin/sleep 10 & echo $! > \"$PID_FILE\""]) + .env("PID_FILE", &pid_file); + + let status = command_status_disclaimed(&command, Duration::from_secs(10)) + .expect("disclaimed command runs"); + let descendant: libc::pid_t = std::fs::read_to_string(&pid_file) + .expect("descendant pid") + .trim() + .parse() + .expect("numeric pid"); + let own = libc::pid_t::try_from(std::process::id()).expect("pid fits"); + let ours = responsible_pid(own).expect("responsibility lookup"); + let theirs = responsible_pid(descendant).expect("responsibility lookup"); + // SAFETY: the descendant was started by this test and only sleeps. + unsafe { libc::kill(descendant, libc::SIGKILL) }; + + assert!(status.success()); + // A disclaimed shell is the descendant's responsible process; after it exits, + // the descendant reports itself instead of inheriting ours. + assert_ne!(theirs, ours); + } +}