From efd4e414a2a20da47b37388af5a2a123f2f5891e Mon Sep 17 00:00:00 2001 From: wangnov <48670012+Wangnov@users.noreply.github.com> Date: Fri, 19 Jun 2026 18:43:40 +0800 Subject: [PATCH] fix: harden update compatibility gates --- crates/codex-mac-engine/Cargo.lock | 2 + crates/codex-mac-engine/Cargo.toml | 1 + crates/codex-mac-engine/src/codesign.rs | 107 +++++++++++++++- crates/codex-win-engine/src/download.rs | 161 ++++++++++++++++++++---- src-tauri/Cargo.lock | 1 + 5 files changed, 242 insertions(+), 30 deletions(-) diff --git a/crates/codex-mac-engine/Cargo.lock b/crates/codex-mac-engine/Cargo.lock index 9ab2cf7..3f55b8c 100644 --- a/crates/codex-mac-engine/Cargo.lock +++ b/crates/codex-mac-engine/Cargo.lock @@ -53,6 +53,8 @@ version = "0.1.0" dependencies = [ "base64", "ed25519-dalek", + "libc", + "log", "roxmltree", "serde", "thiserror", diff --git a/crates/codex-mac-engine/Cargo.toml b/crates/codex-mac-engine/Cargo.toml index 55b1835..80d1acf 100644 --- a/crates/codex-mac-engine/Cargo.toml +++ b/crates/codex-mac-engine/Cargo.toml @@ -13,6 +13,7 @@ ed25519-dalek = "2" base64 = "0.22" log = "0.4" uuid = { version = "1", features = ["v4"] } +libc = "0.2" [[bin]] name = "mac_plan" diff --git a/crates/codex-mac-engine/src/codesign.rs b/crates/codex-mac-engine/src/codesign.rs index 9ed8933..7e0f8d8 100644 --- a/crates/codex-mac-engine/src/codesign.rs +++ b/crates/codex-mac-engine/src/codesign.rs @@ -11,11 +11,74 @@ use crate::EngineError; const CODESIGN: &str = "/usr/bin/codesign"; const SPCTL: &str = "/usr/sbin/spctl"; +const MIN_GATEKEEPER_NOFILE_LIMIT: u64 = 32_768; /// OpenAI's Apple Developer Team ID — verified on a real notarized Codex.app /// (`Developer ID Application: OpenAI OpCo, LLC (2DC432GLL2)`). pub const OPENAI_TEAM_ID: &str = "2DC432GLL2"; +#[cfg(unix)] +fn try_raise_nofile_limit(min_soft_limit: u64) -> Result, String> { + let mut limit = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + let rc = unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit) }; + if rc != 0 { + return Err(std::io::Error::last_os_error().to_string()); + } + + let desired = min_soft_limit as libc::rlim_t; + let target = if limit.rlim_max == libc::RLIM_INFINITY { + desired + } else { + desired.min(limit.rlim_max) + }; + if limit.rlim_cur >= target { + return Ok(None); + } + + let previous = limit.rlim_cur as u64; + limit.rlim_cur = target; + let rc = unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &limit) }; + if rc != 0 { + return Err(std::io::Error::last_os_error().to_string()); + } + Ok(Some((previous, target as u64))) +} + +#[cfg(not(unix))] +fn try_raise_nofile_limit(_min_soft_limit: u64) -> Result, String> { + Ok(None) +} + +fn prepare_gatekeeper_process_limits() { + match try_raise_nofile_limit(MIN_GATEKEEPER_NOFILE_LIMIT) { + Ok(Some((previous, current))) => log::info!( + "raised process file descriptor soft limit for Gatekeeper previous={previous} current={current}" + ), + Ok(None) => log::debug!("process file descriptor soft limit already sufficient for Gatekeeper"), + Err(err) => log::warn!("could not raise process file descriptor soft limit: {err}"), + } +} + +fn is_too_many_open_files(text: &str) -> bool { + text.to_ascii_lowercase().contains("too many open files") +} + +fn gatekeeper_failure_message(app: &Path, stderr: &str) -> String { + let stderr = stderr.trim(); + if is_too_many_open_files(stderr) { + format!( + "Gatekeeper assessment could not complete because macOS reported too many open files while checking {}. No files were replaced; raise the macOS maxfiles limit or close file-heavy apps and retry. raw='{}'", + app.display(), + stderr + ) + } else { + format!("Gatekeeper rejected bundle: {stderr}") + } +} + /// `codesign --verify --deep --strict` — fails if any sealed byte changed. pub fn verify_signature(app: &Path) -> Result<(), EngineError> { let output = Command::new(CODESIGN) @@ -62,15 +125,16 @@ pub fn require_team(app: &Path, expected: &str) -> Result<(), EngineError> { /// `spctl --assess --type execute` — Gatekeeper's verdict (notarization). /// Passes offline when the notarization ticket is stapled (Codex's is). pub fn assess_gatekeeper(app: &Path) -> Result<(), EngineError> { + prepare_gatekeeper_process_limits(); let output = Command::new(SPCTL) .args(["--assess", "--type", "execute"]) .arg(app) .output() .map_err(|e| EngineError::Io(format!("spawn spctl: {e}")))?; if !output.status.success() { - return Err(EngineError::Verify(format!( - "Gatekeeper rejected bundle: {}", - String::from_utf8_lossy(&output.stderr).trim() + return Err(EngineError::Verify(gatekeeper_failure_message( + app, + &String::from_utf8_lossy(&output.stderr), ))); } Ok(()) @@ -108,3 +172,40 @@ pub fn gate_reconstructed(app: &Path) -> Result<(), EngineError> { log::info!("codesign Gatekeeper gate passed path={app_name} team={team}"); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_gatekeeper_file_descriptor_exhaustion() { + assert!(is_too_many_open_files( + "/tmp/Codex.app: Too many open files" + )); + assert!(is_too_many_open_files( + "gatekeeper rejected bundle: too many open files" + )); + assert!(!is_too_many_open_files( + "/tmp/Codex.app: rejected (the code is valid but does not seem to be an app)" + )); + } + + #[test] + fn explains_gatekeeper_resource_failure_without_calling_it_rejected() { + let message = gatekeeper_failure_message( + Path::new("/tmp/Codex.app"), + "/tmp/Codex.app: Too many open files", + ); + + assert!(message.contains("could not complete")); + assert!(message.contains("No files were replaced")); + assert!(!message.contains("rejected bundle")); + } + + #[test] + fn preserves_rejected_wording_for_real_gatekeeper_rejections() { + let message = gatekeeper_failure_message(Path::new("/tmp/Codex.app"), "rejected"); + + assert_eq!(message, "Gatekeeper rejected bundle: rejected"); + } +} diff --git a/crates/codex-win-engine/src/download.rs b/crates/codex-win-engine/src/download.rs index cca7eae..647ecf9 100644 --- a/crates/codex-win-engine/src/download.rs +++ b/crates/codex-win-engine/src/download.rs @@ -15,6 +15,32 @@ static DOWNLOAD_ACTIVE: AtomicBool = AtomicBool::new(false); static DOWNLOAD_CANCELLED: AtomicBool = AtomicBool::new(false); static DOWNLOAD_DISCARD: AtomicBool = AtomicBool::new(false); +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ProgressMode { + NoProgressMeter, + SilentWithErrors, +} + +#[derive(Debug, Eq, PartialEq)] +enum CurlAttemptError { + Cancelled, + Curl { + exit_code: Option, + stderr: String, + }, + Other(String), +} + +impl CurlAttemptError { + fn into_message(self, url: &str) -> String { + match self { + Self::Cancelled => "download cancelled".to_string(), + Self::Curl { exit_code, stderr } => curl_failure_message(url, exit_code, &stderr), + Self::Other(message) => message, + } + } +} + struct DownloadGuard; impl DownloadGuard { @@ -101,80 +127,123 @@ fn curl_failure_message(url: &str, exit_code: Option, stderr: &str) -> Stri ) } -fn run_curl( +fn curl_args( url: &str, - dest: &Path, + dest: &str, resume: bool, - max_bytes: u64, - on_progress: &dyn Fn(u64), -) -> Result<(), String> { - let source = url_host(url); - let dest = dest.to_string_lossy().into_owned(); - let max_bytes = max_bytes.to_string(); + max_bytes: &str, + progress_mode: ProgressMode, +) -> Vec { let mut args = vec![ "-fL".to_string(), "--proto".to_string(), "=https".to_string(), "--proto-redir".to_string(), "=https".to_string(), - "--no-progress-meter".to_string(), + ]; + match progress_mode { + ProgressMode::NoProgressMeter => args.push("--no-progress-meter".to_string()), + ProgressMode::SilentWithErrors => args.push("-sS".to_string()), + } + args.extend([ "--connect-timeout".to_string(), "20".to_string(), "--max-filesize".to_string(), - max_bytes, + max_bytes.to_string(), "--retry".to_string(), "2".to_string(), - ]; + ]); if resume { args.extend(["-C".to_string(), "-".to_string()]); } - args.extend(["-o".to_string(), dest.clone(), url.to_string()]); + args.extend(["-o".to_string(), dest.to_string(), url.to_string()]); + args +} +fn is_no_progress_meter_unsupported(exit_code: Option, stderr: &str) -> bool { + exit_code == Some(2) + && stderr.contains("--no-progress-meter") + && (stderr.contains("is unknown") || stderr.contains("unknown option")) +} + +fn run_curl_once( + source: &str, + dest: &str, + args: Vec, + on_progress: &dyn Fn(u64), +) -> Result<(), CurlAttemptError> { let mut child = hidden_command(curl_exe()) .args(args) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .map_err(|e| format!("spawn curl: {e}"))?; + .map_err(|e| CurlAttemptError::Other(format!("spawn curl: {e}")))?; loop { if DOWNLOAD_CANCELLED.load(Ordering::SeqCst) { let _ = child.kill(); let _ = child.wait(); - let downloaded = std::fs::metadata(&dest).map(|m| m.len()).unwrap_or(0); + let downloaded = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0); log::info!("Windows download cancelled source={source} downloaded={downloaded}"); - return Err("download cancelled".to_string()); + return Err(CurlAttemptError::Cancelled); } - let downloaded = std::fs::metadata(&dest).map(|m| m.len()).unwrap_or(0); + let downloaded = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0); on_progress(downloaded); match child.try_wait() { Ok(Some(_)) => break, Ok(None) => thread::sleep(Duration::from_millis(200)), Err(err) => { let _ = child.kill(); - return Err(format!("wait for curl: {err}")); + return Err(CurlAttemptError::Other(format!("wait for curl: {err}"))); } } } - let output = match child.wait_with_output() { - Ok(output) => output, - Err(err) => return Err(format!("collect curl output: {err}")), - }; + let output = child + .wait_with_output() + .map_err(|err| CurlAttemptError::Other(format!("collect curl output: {err}")))?; if !output.status.success() { - return Err(curl_failure_message( - url, - output.status.code(), - &String::from_utf8_lossy(&output.stderr), - )); + return Err(CurlAttemptError::Curl { + exit_code: output.status.code(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + }); } - let downloaded = std::fs::metadata(&dest).map(|m| m.len()).unwrap_or(0); + let downloaded = std::fs::metadata(dest).map(|m| m.len()).unwrap_or(0); on_progress(downloaded); log::info!("Windows curl download completed source={source} bytes={downloaded}"); Ok(()) } +fn run_curl( + url: &str, + dest: &Path, + resume: bool, + max_bytes: u64, + on_progress: &dyn Fn(u64), +) -> Result<(), String> { + let source = url_host(url); + let dest = dest.to_string_lossy().into_owned(); + let max_bytes = max_bytes.to_string(); + + let modern_args = curl_args(url, &dest, resume, &max_bytes, ProgressMode::NoProgressMeter); + if let Err(first_err) = run_curl_once(source, &dest, modern_args, on_progress) { + if let CurlAttemptError::Curl { exit_code, stderr } = &first_err { + if is_no_progress_meter_unsupported(*exit_code, stderr) { + log::warn!( + "Windows curl does not support --no-progress-meter; retrying with -sS source={source}" + ); + let compat_args = + curl_args(url, &dest, resume, &max_bytes, ProgressMode::SilentWithErrors); + return run_curl_once(source, &dest, compat_args, on_progress) + .map_err(|err| err.into_message(url)); + } + } + return Err(first_err.into_message(url)); + } + Ok(()) +} + pub fn download_to(url: &str, dest: &Path) -> Result<(), EngineError> { download_to_with_progress(url, dest, &|_| {}) } @@ -294,6 +363,44 @@ mod tests { assert!(!cancel_active_download()); } + #[test] + fn curl_args_keep_modern_progress_flag_by_default() { + let args = curl_args( + "https://example.test/Codex.msix", + "Codex.msix.part", + false, + "123", + ProgressMode::NoProgressMeter, + ); + + assert!(args.contains(&"--no-progress-meter".to_string())); + assert!(!args.contains(&"-sS".to_string())); + } + + #[test] + fn curl_args_can_fall_back_to_legacy_silent_mode() { + let args = curl_args( + "https://example.test/Codex.msix", + "Codex.msix.part", + true, + "123", + ProgressMode::SilentWithErrors, + ); + + assert!(args.contains(&"-sS".to_string())); + assert!(!args.contains(&"--no-progress-meter".to_string())); + assert!(args.windows(2).any(|pair| pair == ["-C", "-"])); + } + + #[test] + fn detects_old_curl_without_no_progress_meter() { + let stderr = "curl: option --no-progress-meter: is unknown\ncurl: try 'curl --help' for more information"; + + assert!(is_no_progress_meter_unsupported(Some(2), stderr)); + assert!(!is_no_progress_meter_unsupported(Some(22), stderr)); + assert!(!is_no_progress_meter_unsupported(Some(2), "curl: (6) Could not resolve host")); + } + #[test] fn download_with_progress_reports_final_size() { if hidden_command(curl_exe()) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 116c750..9d9d5d2 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -639,6 +639,7 @@ version = "0.1.0" dependencies = [ "base64 0.22.1", "ed25519-dalek", + "libc", "log", "roxmltree", "serde",