diff --git a/codex-rs/app-server-daemon/src/backend/pid.rs b/codex-rs/app-server-daemon/src/backend/pid.rs index eeb0512850..a24414094d 100644 --- a/codex-rs/app-server-daemon/src/backend/pid.rs +++ b/codex-rs/app-server-daemon/src/backend/pid.rs @@ -1,4 +1,7 @@ -//! PID reservations serialize detached launches; creation times protect stale-record cleanup. +//! PID reservations serialize detached launches; process identities protect stale-record cleanup. + +#[path = "pid_identity.rs"] +mod identity; use std::io::SeekFrom; use std::path::Path; @@ -40,8 +43,12 @@ pub(crate) struct PidBackend { #[serde(rename_all = "camelCase")] struct PidRecord { pid: u32, + // Keep the legacy timestamp for older CLI/updater versions reading this record. process_start_time: String, #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(alias = "linuxProcessIdentity")] + process_identity: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] executable_identity: Option, } @@ -539,20 +546,37 @@ async fn process_matches_record(record: &PidRecord) -> Result { return Ok(false); } - match read_process_details(record.pid).await { - Ok((state, start_time)) => { + let details = async { + #[cfg(any(target_os = "linux", target_os = "macos"))] + if let Some(expected) = &record.process_identity { + return expected.matches_process(record.pid).await; + } + let (state, start_time) = read_process_details(record.pid).await?; + let matches = start_time == record.process_start_time; + let is_zombie = state.starts_with('Z'); + if !matches && !is_zombie { + bail!( + "cannot verify pid-managed process {}: legacy start time changed; PID record retained. \ + Retry with the locale and timezone used to start the daemon. If the system clock \ + changed, stop the original process before restarting the daemon", + record.pid + ); + } + Ok((is_zombie, matches)) + } + .await; + match details { + Ok((is_zombie, matches)) => { // An unreaped zombie still passes kill(pid, 0) and retains its start // time, but it can no longer run the app-server or updater. - if state.starts_with('Z') { - if start_time == record.process_start_time - && let Ok(raw_pid) = libc::pid_t::try_from(record.pid) - { + if is_zombie { + if matches && let Ok(raw_pid) = libc::pid_t::try_from(record.pid) { // Re-exec can lose the Child handle without changing parenthood. unsafe { libc::waitpid(raw_pid, std::ptr::null_mut(), libc::WNOHANG) }; } return Ok(false); } - Ok(start_time == record.process_start_time) + Ok(matches) } Err(_err) if !process_exists(record.pid) => Ok(false), Err(err) => Err(err), @@ -714,21 +738,7 @@ async fn read_process_start_time(pid: u32) -> Result { #[cfg(windows)] async fn process_matches_record(record: &PidRecord) -> Result { - let process = match super::windows::Process::open(record.pid) { - Ok(process) => process, - // A managed daemon is queryable by its launching user. A stale PID may - // have been reused by a protected process; never try to terminate it. - Err(err) - if err.downcast_ref::().is_some_and(|err| { - err.raw_os_error() - == Some(windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED as i32) - }) => - { - return Ok(false); - } - Err(err) => return Err(err), - }; - let Some(process) = process else { + let Some(process) = super::windows::Process::open(record.pid)? else { return Ok(false); }; Ok(process.is_running()? && process.start_time()? == record.process_start_time) diff --git a/codex-rs/app-server-daemon/src/backend/pid_identity.rs b/codex-rs/app-server-daemon/src/backend/pid_identity.rs new file mode 100644 index 0000000000..ea631a3ec2 --- /dev/null +++ b/codex-rs/app-server-daemon/src/backend/pid_identity.rs @@ -0,0 +1,242 @@ +//! Native process identities and zombie state, independent of locale and timezone. +//! Boot IDs prevent records surviving reboot from matching reused process identifiers. + +#[cfg(any(target_os = "linux", target_os = "macos", test))] +use anyhow::Context; +#[cfg(any(target_os = "linux", target_os = "macos", test))] +use anyhow::Result; +use serde::Deserialize; +use serde::Serialize; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged, rename_all_fields = "camelCase")] +pub(super) enum ProcessIdentity { + Linux { + boot_id: String, + start_ticks: u64, + }, + // Untagged decoding must try this before the timestamp-only MacOs variant. + MacOsUnique { + boot_id: String, + unique_id: u64, + start_seconds: u64, + start_microseconds: u64, + }, + MacOs { + start_seconds: u64, + start_microseconds: u64, + }, +} + +#[cfg(any(target_os = "linux", target_os = "macos"))] +impl ProcessIdentity { + pub(super) async fn matches_process(&self, pid: u32) -> Result<(bool, bool)> { + // A different boot proves staleness even when the reused PID is inaccessible. + if let Self::Linux { boot_id, .. } | Self::MacOsUnique { boot_id, .. } = self + && *boot_id != read_boot_id().await? + { + return Ok((false, false)); + } + #[cfg(target_os = "macos")] + if let Self::MacOsUnique { unique_id, .. } = self + && *unique_id != read_unique_id(pid)? + { + return Ok((false, false)); + } + let (is_zombie, actual) = read_process_details(pid).await?; + let matches = if let ( + // Timestamp-only native records predate the cross-user identity check. + Self::MacOs { + start_seconds, + start_microseconds, + }, + Self::MacOsUnique { + start_seconds: actual_seconds, + start_microseconds: actual_microseconds, + .. + }, + ) = (self, &actual) + { + start_seconds == actual_seconds && start_microseconds == actual_microseconds + } else { + *self == actual + }; + Ok((is_zombie, matches)) + } +} + +#[cfg(any(target_os = "linux", target_os = "macos"))] +impl super::PidBackend { + pub(crate) async fn promote_legacy_identity(&self) -> Result<()> { + let _reservation = self.acquire_reservation_lock().await?; + let super::PidFileState::Running(mut record) = + self.read_pid_file_state_with_lock_held().await? + else { + return Ok(()); + }; + if matches!( + record.process_identity, + Some(ProcessIdentity::Linux { .. } | ProcessIdentity::MacOsUnique { .. }) + ) { + return Ok(()); + } + // An updater keeps its PID across exec during upgrades. Only promote a + // legacy record while its original check still identifies the process, + // with the native identity unchanged on both sides of that check. + let before = read_process_details(record.pid).await?; + if before.0 || !super::process_matches_record(&record).await? { + return Ok(()); + } + if read_process_details(record.pid).await? != before { + return Ok(()); + } + record.process_identity = Some(before.1); + let temporary = self.pid_file.with_extension("pid.tmp"); + tokio::fs::write(&temporary, serde_json::to_vec(&record)?).await?; + tokio::fs::rename(&temporary, &self.pid_file).await?; + Ok(()) + } +} + +#[cfg(target_os = "macos")] +pub(super) async fn read_process_details(pid: u32) -> Result<(bool, ProcessIdentity)> { + let boot_id = read_boot_id().await?; + let unique_id = read_unique_id(pid)?; + let pid = libc::pid_t::try_from(pid).context("daemon pid is out of range")?; + let mut info = std::mem::MaybeUninit::::uninit(); + let size = std::mem::size_of::() as libc::c_int; + // SAFETY: the buffer has the exact size and alignment required by this flavor. + // arg=1 includes zombies so callers can reap children that have exited. + let read = unsafe { + libc::proc_pidinfo( + pid, + libc::PROC_PIDTBSDINFO, + /*arg*/ 1, + info.as_mut_ptr().cast(), + size, + ) + }; + if read <= 0 { + return Err(std::io::Error::last_os_error()) + .context("failed to read daemon process identity"); + } + anyhow::ensure!(read == size, "incomplete daemon process identity"); + // SAFETY: proc_pidinfo initialized the entire structure on success. + let info = unsafe { info.assume_init() }; + anyhow::ensure!( + unique_id == read_unique_id(info.pbi_pid)?, + "daemon process changed while reading its identity" + ); + Ok(( + info.pbi_status == libc::SZOMB, + ProcessIdentity::MacOsUnique { + boot_id, + unique_id, + start_seconds: info.pbi_start_tvsec, + start_microseconds: info.pbi_start_tvusec, + }, + )) +} + +#[cfg(target_os = "macos")] +fn read_unique_id(pid: u32) -> Result { + // XNU's proc_uniqidentifierinfo ABI (flavor 17), unchanged in size since 2013. + // Unlike PROC_PIDTBSDINFO, this query does not require matching process ownership. + #[repr(C)] + struct UniqueInfo { + executable_uuid: [u8; 16], + unique_id: u64, + parent_unique_id: u64, + reserved: [u64; 3], + } + let pid = libc::pid_t::try_from(pid).context("daemon pid is out of range")?; + let mut info = std::mem::MaybeUninit::::uninit(); + let size = std::mem::size_of::() as libc::c_int; + // SAFETY: the buffer matches proc_uniqidentifierinfo's size and alignment. + let read = unsafe { + libc::proc_pidinfo( + pid, + /*flavor*/ 17, + /*arg*/ 1, + info.as_mut_ptr().cast(), + size, + ) + }; + if read <= 0 { + return Err(std::io::Error::last_os_error()).context("failed to read daemon unique ID"); + } + anyhow::ensure!(read == size, "incomplete daemon unique ID"); + // SAFETY: proc_pidinfo initialized the entire structure on success. + Ok(unsafe { info.assume_init() }.unique_id) +} + +#[cfg(target_os = "macos")] +async fn read_boot_id() -> Result { + let mut buffer = [0u8; 37]; + let mut size = buffer.len(); + // SAFETY: the writable buffer has size bytes; the new-value pointer is null. + let result = unsafe { + libc::sysctlbyname( + c"kern.bootsessionuuid".as_ptr(), + buffer.as_mut_ptr().cast(), + &mut size, + std::ptr::null_mut(), + /*newlen*/ 0, + ) + }; + if result != 0 { + return Err(std::io::Error::last_os_error()).context("failed to read macOS boot ID"); + } + anyhow::ensure!(size == buffer.len(), "incomplete macOS boot ID"); + Ok(std::ffi::CStr::from_bytes_with_nul(&buffer)? + .to_str()? + .to_owned()) +} + +#[cfg(target_os = "linux")] +pub(super) async fn read_process_details(pid: u32) -> Result<(bool, ProcessIdentity)> { + let stat = tokio::fs::read(format!("/proc/{pid}/stat")) + .await + .with_context(|| format!("failed to read process stat for pid-managed process {pid}"))?; + let (state, start_ticks) = parse_stat(&stat)?; + Ok(( + state == "Z", + ProcessIdentity::Linux { + boot_id: read_boot_id().await?, + start_ticks, + }, + )) +} + +#[cfg(target_os = "linux")] +async fn read_boot_id() -> Result { + let boot_id = tokio::fs::read_to_string("/proc/sys/kernel/random/boot_id") + .await + .context("failed to read Linux boot ID")?; + let boot_id = boot_id.trim(); + anyhow::ensure!(!boot_id.is_empty(), "Linux boot ID is empty"); + Ok(boot_id.to_owned()) +} + +#[cfg(any(target_os = "linux", test))] +fn parse_stat(stat: &[u8]) -> Result<(String, u64)> { + // comm (field 2) can contain spaces and closing parentheses. The final ')' + // terminates it; splitting the whole line on whitespace miscounts fields. + let end = stat + .iter() + .rposition(|byte| *byte == b')') + .context("process stat has no comm")?; + let fields = std::str::from_utf8(&stat[end + 1..]).context("invalid process stat fields")?; + let mut fields = fields.split_whitespace(); + let state = fields.next().context("process stat has no state")?; + let start_ticks = fields + .nth(/*n*/ 18) + .context("process stat has no start time")? + .parse() + .context("process stat start time is invalid")?; + Ok((state.to_string(), start_ticks)) +} + +#[cfg(test)] +#[path = "pid_identity_tests.rs"] +mod tests; diff --git a/codex-rs/app-server-daemon/src/backend/pid_identity_tests.rs b/codex-rs/app-server-daemon/src/backend/pid_identity_tests.rs new file mode 100644 index 0000000000..0454e529e0 --- /dev/null +++ b/codex-rs/app-server-daemon/src/backend/pid_identity_tests.rs @@ -0,0 +1,89 @@ +use super::parse_stat; +use pretty_assertions::assert_eq; + +#[test] +fn parses_start_ticks_after_comm_with_spaces_and_parentheses() { + let stat = + b"123 (codex \xff) worker) S 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 987654321 20"; + assert_eq!(parse_stat(stat).unwrap(), ("S".to_string(), 987654321)); +} + +#[test] +fn reads_earlier_linux_identity_field() { + use crate::backend::pid::PidRecord; + let mut expected = serde_json::json!({ + "pid": 42, + "processStartTime": "legacy timestamp", + "linuxProcessIdentity": {"bootId": "boot", "startTicks": 123}, + }); + let record: PidRecord = serde_json::from_value(expected.clone()).unwrap(); + expected["processIdentity"] = expected + .as_object_mut() + .unwrap() + .remove("linuxProcessIdentity") + .unwrap(); + assert_eq!(serde_json::to_value(record).unwrap(), expected); +} + +#[cfg(any(target_os = "linux", target_os = "macos"))] +#[tokio::test] +async fn previous_boot_does_not_require_process_inspection() { + let (_, identity) = super::read_process_details(std::process::id()) + .await + .unwrap(); + let mut record = serde_json::to_value(identity).unwrap(); + record["bootId"] = "previous boot".into(); + let identity: super::ProcessIdentity = serde_json::from_value(record).unwrap(); + // An invalid PID would make process inspection fail if it preceded the boot check. + assert_eq!( + identity.matches_process(u32::MAX).await.unwrap(), + (false, false) + ); +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn reused_pid_can_belong_to_root() { + let (_, identity) = super::read_process_details(std::process::id()) + .await + .unwrap(); + // launchd is root-owned: PROC_PIDTBSDINFO returns EPERM for an ordinary caller. + assert_eq!( + identity.matches_process(/*pid*/ 1).await.unwrap(), + (false, false) + ); +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn promotes_timestamp_only_macos_record() { + use crate::backend::pid::PidBackend; + use crate::backend::pid::PidRecord; + + let temp = tempfile::TempDir::new().unwrap(); + let path = temp.path().join("server.pid"); + let (_, identity) = super::read_process_details(std::process::id()) + .await + .unwrap(); + let mut legacy = serde_json::to_value(&identity).unwrap(); + legacy.as_object_mut().unwrap().remove("bootId"); + legacy.as_object_mut().unwrap().remove("uniqueId"); + let mut record = PidRecord { + pid: std::process::id(), + process_start_time: "unused legacy text".into(), + process_identity: Some(serde_json::from_value(legacy).unwrap()), + executable_identity: None, + }; + std::fs::write(&path, serde_json::to_vec(&record).unwrap()).unwrap(); + let backend = PidBackend::new( + temp.path().join("codex"), + path.clone(), + /*remote_control_enabled*/ false, + ); + backend.promote_legacy_identity().await.unwrap(); + record.process_identity = Some(identity); + assert_eq!( + serde_json::from_slice::(&std::fs::read(path).unwrap()).unwrap(), + record, + ); +} diff --git a/codex-rs/app-server-daemon/src/backend/pid_start.rs b/codex-rs/app-server-daemon/src/backend/pid_start.rs index 55ab8fd12b..35248a7fd1 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_start.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_start.rs @@ -260,15 +260,24 @@ impl PidBackend { super::super::windows::Process::open(pid)? .context("daemon exited during launch")? .ensure_detached()?; - read_process_start_time(pid).await + let process_start_time = read_process_start_time(pid).await?; + #[cfg(any(target_os = "linux", target_os = "macos"))] + let process_identity = super::identity::read_process_details(pid) + .await + .ok() + .map(|(_, identity)| identity); + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + let process_identity = None; + anyhow::Ok(PidRecord { + pid, + process_start_time, + process_identity, + executable_identity: launched_identity, + }) } .await { - Ok(process_start_time) => PidRecord { - pid, - process_start_time, - executable_identity: launched_identity, - }, + Ok(record) => record, Err(err) => { let _ = self.terminate_process(pid); let mut context = diff --git a/codex-rs/app-server-daemon/src/backend/pid_tests.rs b/codex-rs/app-server-daemon/src/backend/pid_tests.rs index 36f1b30650..5924495f64 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_tests.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_tests.rs @@ -197,11 +197,13 @@ async fn stale_record_cleanup_preserves_replacement_record() { let stale = PidRecord { pid: 1, process_start_time: "old".to_string(), + process_identity: None, executable_identity: None, }; let replacement = PidRecord { pid: 2, process_start_time: "new".to_string(), + process_identity: None, executable_identity: None, }; tokio::fs::write( @@ -276,6 +278,38 @@ async fn pid_record_captures_the_resolved_launch_binary() { backend.stop().await.expect("stop daemon"); } +#[cfg(unix)] +#[tokio::test] +async fn legacy_start_time_mismatch_preserves_record_and_process() { + let temp = TempDir::new().unwrap(); + let backend = PidBackend::new( + temp.path().join("codex"), + temp.path().join("app-server.pid"), + /*remote_control_enabled*/ false, + ); + let contents = serde_json::to_vec(&serde_json::json!({ + "pid": std::process::id(), + "processStartTime": "historical wall-clock start time", + })) + .unwrap(); + std::fs::write(&backend.pid_file, &contents).unwrap(); + for result in [ + backend.is_starting_or_running().await.map(|_| ()), + backend.start().await.map(|_| ()), + backend.stop().await, + #[cfg(any(target_os = "linux", target_os = "macos"))] + backend.promote_legacy_identity().await, + ] { + assert!( + result + .unwrap_err() + .to_string() + .contains("PID record retained") + ); + } + assert_eq!(std::fs::read(&backend.pid_file).unwrap(), contents); +} + #[cfg(unix)] #[tokio::test] async fn stop_reaps_untracked_app_server_child() { @@ -292,6 +326,7 @@ async fn stop_reaps_untracked_app_server_child() { let record = PidRecord { pid, process_start_time: read_process_start_time(pid).await.expect("start time"), + process_identity: None, executable_identity: None, }; tokio::fs::write( @@ -383,6 +418,7 @@ async fn shutdown_grace_handles_process_exit() { process_start_time: super::read_process_start_time(pid) .await .expect("start time"), + process_identity: None, executable_identity: None, }; tokio::fs::write( @@ -460,6 +496,7 @@ async fn stopping_updater_signals_its_installer_process_group() { serde_json::to_vec(&PidRecord { pid, process_start_time: read_process_start_time(pid).await.expect("start time"), + process_identity: None, executable_identity: None, }) .expect("serialize pid"), @@ -488,11 +525,21 @@ async fn exited_unreaped_updater_is_reaped() { .arg("60") .spawn() .expect("spawn updater shim"); + #[cfg(any(target_os = "linux", target_os = "macos"))] + let process_identity = Some( + super::identity::read_process_details(child.id()) + .await + .unwrap() + .1, + ); + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + let process_identity = None; let record = PidRecord { pid: child.id(), process_start_time: read_process_start_time(child.id()) .await .expect("start time"), + process_identity, executable_identity: None, }; let backend = @@ -584,7 +631,6 @@ async fn read_stderr_log_tail_returns_recent_complete_lines() { ); } -#[cfg(windows)] #[tokio::test] async fn stale_creation_time_never_stops_reused_pid() { let temp = TempDir::new().expect("temp"); @@ -593,17 +639,45 @@ async fn stale_creation_time_never_stops_reused_pid() { temp.path().join("server.pid"), /*remote_control_enabled*/ false, ); - let record = PidRecord { - pid: std::process::id(), - process_start_time: "stale".into(), - executable_identity: None, + #[cfg(any(target_os = "linux", target_os = "macos"))] + let identities = { + use super::identity::ProcessIdentity; + let (_, identity) = super::identity::read_process_details(std::process::id()) + .await + .unwrap(); + let mut stale_time = identity.clone(); + let mut stale_epoch = identity; + match &mut stale_time { + ProcessIdentity::Linux { start_ticks, .. } => *start_ticks += 1, + ProcessIdentity::MacOs { + start_microseconds, .. + } + | ProcessIdentity::MacOsUnique { + start_microseconds, .. + } => *start_microseconds += 1, + } + match &mut stale_epoch { + ProcessIdentity::Linux { boot_id, .. } => boot_id.push_str("-previous"), + ProcessIdentity::MacOs { start_seconds, .. } => *start_seconds += 1, + ProcessIdentity::MacOsUnique { unique_id, .. } => *unique_id += 1, + } + [Some(stale_time), Some(stale_epoch)] }; - tokio::fs::write(&backend.pid_file, serde_json::to_vec(&record).unwrap()) - .await - .unwrap(); - backend.stop().await.expect("stale record cleanup"); - assert!(!backend.pid_file.exists()); - assert!(!backend.pid_file.with_extension("shutdown").exists()); + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + let identities = [None]; + for process_identity in identities { + let record = PidRecord { + pid: std::process::id(), + process_start_time: "stale".into(), + process_identity, + executable_identity: None, + }; + let contents = serde_json::to_vec(&record).unwrap(); + std::fs::write(&backend.pid_file, &contents).unwrap(); + backend.stop().await.expect("stale record cleanup"); + assert!(!backend.pid_file.exists()); + assert!(!backend.pid_file.with_extension("shutdown").exists()); + } } #[cfg(windows)] @@ -624,6 +698,7 @@ async fn failed_updater_handoff_preserves_predecessor_record() { process_start_time: super::read_process_start_time(std::process::id()) .await .unwrap(), + process_identity: None, executable_identity: None, }; for record in [ @@ -685,6 +760,7 @@ async fn updater_readiness_and_post_publication_failure_preserve_ownership() { process_start_time: super::read_process_start_time(pid) .await .expect("creation time"), + process_identity: None, executable_identity: None, }; let predecessor = PidRecord { @@ -692,6 +768,7 @@ async fn updater_readiness_and_post_publication_failure_preserve_ownership() { process_start_time: super::read_process_start_time(std::process::id()) .await .expect("creation time"), + process_identity: None, executable_identity: None, }; tokio::fs::write(&backend.pid_file, serde_json::to_vec(&successor).unwrap()) @@ -779,7 +856,7 @@ async fn updater_readiness_and_post_publication_failure_preserve_ownership() { #[cfg(windows)] #[test] -fn inaccessible_reused_pid_is_stale_without_hiding_process_open_errors() { +fn inaccessible_pid_preserves_identity_check_error() { use futures::FutureExt; use windows_sys::Win32::Security::ImpersonateAnonymousToken; use windows_sys::Win32::Security::RevertToSelf; @@ -791,6 +868,7 @@ fn inaccessible_reused_pid_is_stale_without_hiding_process_open_errors() { let record = PidRecord { pid: std::process::id(), process_start_time: "stale".into(), + process_identity: None, executable_identity: None, }; assert!( @@ -816,7 +894,15 @@ fn inaccessible_reused_pid_is_stale_without_hiding_process_open_errors() { .raw_os_error(), Some(windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED as i32), ); - assert!(!matches.expect("identity check must not suspend").unwrap()); + assert_eq!( + matches + .expect("identity check must not suspend") + .unwrap_err() + .downcast_ref::() + .unwrap() + .raw_os_error(), + Some(windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED as i32), + ); }) .join() .expect("anonymous identity check"); diff --git a/codex-rs/app-server-daemon/src/backend/pid_windows.rs b/codex-rs/app-server-daemon/src/backend/pid_windows.rs index 58150527a1..8487fa2bc4 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_windows.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_windows.rs @@ -42,6 +42,7 @@ impl PidBackend { let record = PidRecord { pid: std::process::id(), process_start_time: read_process_start_time(std::process::id()).await?, + process_identity: None, executable_identity: None, }; self.start_inner(Some(record)).await?; diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 4513ce6855..e8a248f6a5 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -32,7 +32,7 @@ use crate::RestartMode; use crate::managed_install::ExecutableIdentity; use crate::managed_install::executable_identity; use crate::managed_install::resolved_managed_codex_bin; -#[cfg(windows)] +#[cfg(any(target_os = "linux", target_os = "macos", windows))] use crate::settings::DaemonSettings; use crate::settings::UpdaterSettings; @@ -88,6 +88,17 @@ async fn run_with_http( #[cfg(unix)] let mut terminate = signal(SignalKind::terminate()).context("failed to install updater shutdown handler")?; + #[cfg(any(target_os = "linux", target_os = "macos"))] + { + let paths = daemon.backend_paths(&DaemonSettings::default()); + for backend in [ + crate::backend::pid_backend(paths.clone()), + crate::backend::pid_update_loop_backend(paths), + ] { + // Inspection failures must preserve legacy records without blocking updates. + let _ = backend.promote_legacy_identity().await; + } + } #[cfg(windows)] let updater = { // Updater ownership needs only paths, not settings that may be mid-edit. diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index fb923a82bc..666791fc05 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -122,6 +122,99 @@ fn wait_for_exit(pid: u32) -> Result<()> { } } +#[cfg(any(target_os = "linux", target_os = "macos"))] +#[test] +fn managed_identity_survives_locale_and_timezone_changes() -> Result<()> { + let daemon = TestDaemon::new()?; + let mut original = Vec::new(); + let mut updater_pid = 0; + let pid_file = daemon.home.path().join("app-server-daemon/app-server.pid"); + for (action, locale, timezone, expected_status) in [ + ("start", "C", "UTC0", "started"), + ("version", "en_AU.UTF-8", "PST8PDT", "running"), + ("restart", "en_AU.UTF-8", "PST8PDT", "restarted"), + ("stop", "C", "UTC0", "stopped"), + ] { + let output = daemon + .command() + .args(["app-server", "daemon", action]) + .env("LC_ALL", locale) + .env("TZ", timezone) + .output()?; + ensure!( + output.status.success(), + "daemon {action} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let output: Value = serde_json::from_slice(&output.stdout)?; + assert_eq!(output["status"], expected_status); + if action == "start" { + updater_pid = daemon.pid("app-server-updater.pid")?; + original = std::fs::read(&pid_file)?; + let record: Value = serde_json::from_slice(&original)?; + assert!(record["processIdentity"].is_object()); + // Older clients still receive the original, unnormalized ps value. + let legacy = Command::new("ps") + .args([ + "-p", + &daemon.pid("app-server.pid")?.to_string(), + "-o", + "lstart=", + ]) + .env("LC_ALL", locale) + .env("TZ", timezone) + .output()?; + ensure!(legacy.status.success(), "failed to read legacy start time"); + assert_eq!( + record["processStartTime"], + String::from_utf8(legacy.stdout)?.trim() + ); + } else if action == "version" { + assert_eq!(output["backend"], "pid"); + assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid); + assert_eq!(std::fs::read(&pid_file)?, original); + + // Finish startup promotion before deliberately restoring a legacy record. + let updater_socket = daemon + .home + .path() + .join("app-server-daemon/app-server-updater.sock"); + let deadline = Instant::now() + Duration::from_secs(30); + while !updater_socket.exists() { + ensure!(Instant::now() < deadline, "updater did not become ready"); + std::thread::sleep(Duration::from_millis(50)); + } + let mut legacy: Value = serde_json::from_slice(&original)?; + legacy + .as_object_mut() + .context("PID record object")? + .remove("processIdentity"); + let legacy_bytes = serde_json::to_vec(&legacy)?; + std::fs::write(&pid_file, &legacy_bytes)?; + let result = daemon + .command() + .args(["app-server", "daemon", "version"]) + .env("LC_ALL", locale) + .env("TZ", timezone) + .output(); + let preserved = std::fs::read(&pid_file); + // Restore the native identity before assertions so Drop can stop the daemon. + std::fs::write(&pid_file, &original)?; + let result = result?; + assert!(!result.status.success()); + assert_eq!(preserved?, legacy_bytes); + let stderr = + String::from_utf8(result.stderr)?.replace(&legacy["pid"].to_string(), "[PID]"); + insta::assert_snapshot!(stderr, @"Error: cannot verify pid-managed process [PID]: legacy start time changed; PID record retained. Retry with the locale and timezone used to start the daemon. If the system clock changed, stop the original process before restarting the daemon"); + } + if action == "restart" { + assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid); + } + } + assert!(!pid_file.exists()); + Ok(()) +} + #[test] fn package_ownership_check_does_not_start_an_updater() -> Result<()> { let daemon = TestDaemon::new()?; @@ -159,10 +252,30 @@ fn managed_starts_ensure_one_updater_and_recover_a_missing_one() -> Result<()> { signal(updater_pid, libc::SIGTERM)?; wait_for_exit(updater_pid)?; + // A replacement updater also upgrades still-verifiable records left by an old CLI. + let server_record_path = daemon.home.path().join("app-server-daemon/app-server.pid"); + let mut legacy: Value = serde_json::from_slice(&std::fs::read(&server_record_path)?)?; + let native = legacy.as_object_mut().unwrap().remove("processIdentity"); + std::fs::write(&server_record_path, serde_json::to_vec(&legacy)?)?; assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning"); assert_eq!(daemon.pid("app-server.pid")?, backend_pid); let replacement_pid = daemon.pid("app-server-updater.pid")?; assert_ne!(replacement_pid, updater_pid); + if let Some(native) = native { + legacy["processIdentity"] = native; + let deadline = Instant::now() + Duration::from_secs(30); + loop { + let record: Value = serde_json::from_slice(&std::fs::read(&server_record_path)?)?; + if record == legacy { + break; + } + ensure!( + Instant::now() < deadline, + "legacy record was not upgraded: {record}" + ); + std::thread::sleep(Duration::from_millis(50)); + } + } assert_eq!(daemon.lifecycle("restart")?["status"], "restarted"); assert_ne!(daemon.pid("app-server.pid")?, backend_pid); assert_eq!(daemon.pid("app-server-updater.pid")?, replacement_pid); @@ -199,6 +312,35 @@ fn managed_start_succeeds_when_updater_record_is_invalid() -> Result<()> { Ok(()) } +#[cfg(any(target_os = "linux", target_os = "macos"))] +#[test] +fn wall_clock_shift_keeps_server_and_updater_managed() -> Result<()> { + let daemon = TestDaemon::new()?; + assert_eq!(daemon.lifecycle("start")?["status"], "started"); + let server_pid = daemon.pid("app-server.pid")?; + let updater_pid = daemon.pid("app-server-updater.pid")?; + for name in ["app-server.pid", "app-server-updater.pid"] { + let path = daemon.home.path().join("app-server-daemon").join(name); + let mut record: Value = serde_json::from_slice(&std::fs::read(&path)?)?; + record["processStartTime"] = "historical wall-clock start time".into(); + let replacement = path.with_extension("replacement"); + std::fs::write(&replacement, serde_json::to_vec(&record)?)?; + std::fs::rename(replacement, path)?; + } + + let version = daemon.lifecycle("version")?; + assert_eq!(version["status"], "running"); + assert_eq!(version["backend"], "pid"); + assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning"); + assert_eq!(daemon.pid("app-server.pid")?, server_pid); + assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid); + assert_eq!(daemon.lifecycle("restart")?["status"], "restarted"); + wait_for_exit(server_pid)?; + assert_ne!(daemon.pid("app-server.pid")?, server_pid); + assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid); + Ok(()) +} + #[test] fn managed_start_keeps_updater_on_marker_mismatch_but_stops_it_for_pin() -> Result<()> { let daemon = TestDaemon::new()?;