diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 01a121bf78..a24fb43638 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2134,6 +2134,7 @@ dependencies = [ "codex-utils-cli", "codex-utils-json-to-toml", "codex-utils-path-uri", + "codex-utils-process", "codex-utils-pty", "codex-web-search-extension", "codex-websocket-auth", @@ -2550,6 +2551,7 @@ dependencies = [ "codex-utils-cli", "codex-utils-path", "codex-utils-path-uri", + "codex-utils-process", "codex-utils-pty", "codex-utils-redacted-string", "codex-websocket-auth", @@ -2965,6 +2967,7 @@ dependencies = [ "codex-utils-path", "codex-utils-path-uri", "codex-utils-plugins", + "codex-utils-process", "codex-utils-pty", "codex-utils-redacted-string", "codex-utils-stream-parser", @@ -3084,6 +3087,7 @@ dependencies = [ "codex-utils-path", "codex-utils-path-uri", "codex-utils-plugins", + "codex-utils-process", "dirs", "flate2", "futures", @@ -3201,6 +3205,7 @@ dependencies = [ "codex-utils-cargo-bin", "codex-utils-home-dir", "codex-utils-path-uri", + "codex-utils-process", "codex-utils-pty", "codex-utils-redacted-string", "codex-utils-rustls-provider", @@ -3459,6 +3464,7 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-path-uri", + "codex-utils-process", "codex-utils-pty", "futures", "gix", @@ -3634,6 +3640,7 @@ dependencies = [ "codex-utils-absolute-path", "codex-utils-output-truncation", "codex-utils-path-uri", + "codex-utils-process", "codex-utils-pty", "futures", "libc", @@ -3794,6 +3801,7 @@ dependencies = [ "codex-protocol", "codex-secrets", "codex-terminal-detection", + "codex-utils-process", "codex-utils-template", "codex-workload-identity", "core_test_support", @@ -4005,6 +4013,7 @@ dependencies = [ "codex-protocol", "codex-response-debug-context", "codex-secrets", + "codex-utils-process", "codex-utils-redacted-string", "http 1.4.0", "pretty_assertions", @@ -4409,6 +4418,7 @@ dependencies = [ "codex-state", "codex-utils-absolute-path", "codex-utils-path", + "codex-utils-process", "pretty_assertions", "regex", "serde", @@ -5064,6 +5074,14 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-utils-process" +version = "0.0.0" +dependencies = [ + "pretty_assertions", + "tokio", +] + [[package]] name = "codex-utils-pty" version = "0.0.0" @@ -5254,6 +5272,7 @@ dependencies = [ "codex-otel", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-process", "codex-utils-pty", "codex-utils-string", "dirs-next", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index adceeb5ed9..a8246dbcb1 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -125,6 +125,7 @@ members = [ "utils/image", "utils/json-to-toml", "utils/home-dir", + "utils/process", "utils/pty", "utils/readiness", "utils/redacted-string", @@ -300,6 +301,7 @@ codex-utils-output-truncation = { path = "utils/output-truncation" } codex-utils-path = { path = "utils/path-utils" } codex-utils-path-uri = { path = "utils/path-uri" } codex-utils-plugins = { path = "utils/plugins" } +codex-utils-process = { path = "utils/process" } codex-utils-pty = { path = "utils/pty" } codex-utils-redacted-string = { path = "utils/redacted-string" } codex-utils-rustls-provider = { path = "utils/rustls-provider" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index ad0a5fbab3..8834c54b7e 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -25,6 +25,7 @@ doctest = false workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } base64 = { workspace = true } axum = { workspace = true, default-features = false, features = [ diff --git a/codex-rs/app-server/src/request_processors/feedback_doctor_report.rs b/codex-rs/app-server/src/request_processors/feedback_doctor_report.rs index 905f721f1d..b01e490b83 100644 --- a/codex-rs/app-server/src/request_processors/feedback_doctor_report.rs +++ b/codex-rs/app-server/src/request_processors/feedback_doctor_report.rs @@ -106,7 +106,7 @@ pub(crate) async fn doctor_feedback_report( } fn doctor_command(executable: &Path, cwd: &Path, codex_home: &Path) -> Command { - let mut command = Command::new(executable); + let mut command = Command::from(codex_utils_process::background_command(executable)); command .arg("--cd") .arg(cwd) diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 4fed7668d3..399230d3f4 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -23,6 +23,7 @@ doctest = false workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["derive", "env"] } diff --git a/codex-rs/cli/src/doctor/desktop/windows_security.rs b/codex-rs/cli/src/doctor/desktop/windows_security.rs index 94e95edca8..d54f1b5936 100644 --- a/codex-rs/cli/src/doctor/desktop/windows_security.rs +++ b/codex-rs/cli/src/doctor/desktop/windows_security.rs @@ -72,7 +72,7 @@ async fn query_channel(wevtutil: &Path, channel: Channel) -> Option>() .join(" or "); - let mut child = Command::new(wevtutil) + let mut child = Command::from(codex_utils_process::background_command(wevtutil)) .args(["qe", channel.1]) .arg(format!( "/q:*[System[({events}) and TimeCreated[timediff(@SystemTime) <= 604800000]]]" diff --git a/codex-rs/cli/src/doctor/security.rs b/codex-rs/cli/src/doctor/security.rs index 0413f9c0a2..64141dd976 100644 --- a/codex-rs/cli/src/doctor/security.rs +++ b/codex-rs/cli/src/doctor/security.rs @@ -236,7 +236,7 @@ async fn endpoint_products() -> EndpointInspection { #[cfg(any(target_os = "macos", target_os = "windows"))] async fn product_command(program: impl AsRef, args: &[&str]) -> Option { - let mut command = Command::new(program); + let mut command = Command::from(codex_utils_process::background_command(program)); command .args(args) .stdin(Stdio::null()) diff --git a/codex-rs/cli/tests/daemon_startup.rs b/codex-rs/cli/tests/daemon_startup.rs index be8752836e..67eff23d4d 100644 --- a/codex-rs/cli/tests/daemon_startup.rs +++ b/codex-rs/cli/tests/daemon_startup.rs @@ -73,8 +73,17 @@ async fn restrictive_launcher_uses_embedded_if_daemon_cannot_start() -> Result<( .arg("--no-alt-screen") .stdin(std::process::Stdio::inherit()) .stdout(std::process::Stdio::inherit()) - .stderr(std::process::Stdio::inherit()); - let status = job.spawn_contained(&mut command)?.wait().await?; + .stderr(std::process::Stdio::inherit()) + // This interactive CLI must inherit the PTY console rather than use + // the background helper's CREATE_NO_WINDOW launch policy. + .creation_flags(windows_sys::Win32::System::Threading::CREATE_SUSPENDED) + .kill_on_drop(true); + let mut child = command.spawn()?; + ensure!( + job.assign_and_resume_process(child.id().context("missing CLI pid")?)?, + "CLI job assignment failed" + ); + let status = child.wait().await?; ensure!(status.success(), "CLI exited: {status}"); return Ok(()); } diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index af58b8e9e8..df9ec2752a 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } codex-analytics = { workspace = true } codex-app-server-protocol = { workspace = true } diff --git a/codex-rs/core-plugins/src/git_policy.rs b/codex-rs/core-plugins/src/git_policy.rs index 27ba6998ae..35feafd0bd 100644 --- a/codex-rs/core-plugins/src/git_policy.rs +++ b/codex-rs/core-plugins/src/git_policy.rs @@ -37,7 +37,7 @@ impl PluginGitMode { /// Creates a Git command with the repository-environment policy for this operation. /// Automatic remote lookups and initial clones must also select a trusted repository. pub(crate) fn command(self, git_binary: &Path) -> Command { - let mut command = Command::new(git_binary); + let mut command = codex_utils_process::background_command(git_binary); command.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG]); if matches!(self, Self::Automatic) { command.env("GIT_OPTIONAL_LOCKS", "0"); diff --git a/codex-rs/core-plugins/src/marketplace_add/install.rs b/codex-rs/core-plugins/src/marketplace_add/install.rs index 84cb7d973e..2a5532ca0c 100644 --- a/codex-rs/core-plugins/src/marketplace_add/install.rs +++ b/codex-rs/core-plugins/src/marketplace_add/install.rs @@ -2,7 +2,6 @@ use super::MarketplaceAddError; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::process::Command; pub(super) fn clone_git_source( url: &str, @@ -111,7 +110,7 @@ pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { } fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), MarketplaceAddError> { - let mut command = Command::new("git"); + let mut command = codex_utils_process::background_command("git"); command .args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG]) .args(args); diff --git a/codex-rs/core-plugins/src/npm_source.rs b/codex-rs/core-plugins/src/npm_source.rs index 3d728bd0f7..cb418edc76 100644 --- a/codex-rs/core-plugins/src/npm_source.rs +++ b/codex-rs/core-plugins/src/npm_source.rs @@ -5,7 +5,6 @@ use std::ffi::OsStr; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::process::Command; use tempfile::TempDir; const NPM_PLUGIN_SOURCE_STAGING_DIR: &str = "plugins/.marketplace-plugin-source-staging"; @@ -87,7 +86,7 @@ fn pack_npm_package( || package.to_string(), |version| format!("{package}@{version}"), ); - let mut command = Command::new(npm_command); + let mut command = codex_utils_process::background_command(npm_command); command .current_dir(destination) .arg("pack") diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 693fbd7b78..d5fd95d2de 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -12,6 +12,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } arc-swap = { workspace = true } async-channel = { workspace = true } diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 596bf8de5a..994506d24c 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -1067,7 +1067,7 @@ async fn run_script_with_timeout( // Handler is kept as guard to control the drop. The `mut` pattern is required because .args() // returns a ref of handler. - let mut handler = Command::new(&args[0]); + let mut handler = Command::from(codex_utils_process::background_command(&args[0])); handler.args(&args[1..]); handler.stdin(Stdio::null()); handler.current_dir(cwd); diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index bbae9308d7..79526b8bfd 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -67,7 +67,12 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io "spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {network_sandbox_policy:?} {stdio_policy:?} {env:?}" ); - let mut cmd = Command::new(&program); + let mut cmd = match stdio_policy { + StdioPolicy::RedirectForShellTool => { + Command::from(codex_utils_process::background_command(&program)) + } + StdioPolicy::Inherit => Command::new(&program), + }; #[cfg(unix)] cmd.arg0(arg0.map_or_else(|| program.to_string_lossy().to_string(), String::from)); cmd.args(args); diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index fd9928db36..3c78a5021c 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -19,6 +19,7 @@ ignored-paths = [ workspace = true [dependencies] +codex-utils-process = { workspace = true } arc-swap = { workspace = true } axum = { workspace = true, features = ["http1", "tokio", "ws"] } base64 = { workspace = true } diff --git a/codex-rs/exec-server/src/client_transport.rs b/codex-rs/exec-server/src/client_transport.rs index a8a5d7967e..beb47a1c37 100644 --- a/codex-rs/exec-server/src/client_transport.rs +++ b/codex-rs/exec-server/src/client_transport.rs @@ -838,7 +838,9 @@ fn is_rendezvous_harness_url(websocket_url: &str) -> bool { } fn stdio_command_process(stdio_command: &StdioExecServerCommand) -> Command { - let mut command = Command::new(&stdio_command.program); + let mut command = Command::from(codex_utils_process::background_command( + &stdio_command.program, + )); command.args(&stdio_command.args); command.envs(&stdio_command.env); scrub_non_inheritable_env_vars(command.as_std_mut()); diff --git a/codex-rs/exec-server/src/connection.rs b/codex-rs/exec-server/src/connection.rs index 476aaf3619..e18faffd1e 100644 --- a/codex-rs/exec-server/src/connection.rs +++ b/codex-rs/exec-server/src/connection.rs @@ -252,7 +252,7 @@ fn kill_direct_child(child_process: &mut Child, action: &str) { #[cfg(windows)] fn kill_windows_process_tree(pid: u32) -> bool { let pid = pid.to_string(); - match std::process::Command::new("taskkill") + match codex_utils_process::background_command("taskkill") .args(["/PID", pid.as_str(), "/T", "/F"]) .stdin(Stdio::null()) .stdout(Stdio::null()) diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index a0bd787285..41f3147685 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -9,6 +9,7 @@ readme = "README.md" workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true } codex-file-system = { workspace = true } diff --git a/codex-rs/git-utils/src/git_process.rs b/codex-rs/git-utils/src/git_process.rs index 3ce0431434..80c3d18599 100644 --- a/codex-rs/git-utils/src/git_process.rs +++ b/codex-rs/git-utils/src/git_process.rs @@ -41,16 +41,7 @@ fn spawn_git_command(command: &mut Command) -> Option<(Child, KillGitProcessTree .stderr(Stdio::piped()); #[cfg(windows)] - let (child, job) = match JobObject::create() - .and_then(|job| job.spawn_contained(command).map(|child| (child, job))) - { - Ok((child, job)) => (child, Some(job)), - Err(_) => { - // A failed contained spawn leaves CREATE_SUSPENDED on the command. - command.creation_flags(0); - (command.spawn().ok()?, None) - } - }; + let (child, job) = JobObject::spawn_background(command).ok()?; #[cfg(not(windows))] let child = command.spawn().ok()?; diff --git a/codex-rs/git-utils/src/operations.rs b/codex-rs/git-utils/src/operations.rs index ba3196da58..f9b9e4b026 100644 --- a/codex-rs/git-utils/src/operations.rs +++ b/codex-rs/git-utils/src/operations.rs @@ -2,7 +2,6 @@ use std::ffi::OsStr; use std::ffi::OsString; use std::path::Path; use std::path::PathBuf; -use std::process::Command; use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; @@ -130,7 +129,7 @@ where args_vec.push(OsString::from(arg.as_ref())); } let command_string = build_command_string(&args_vec); - let mut command = Command::new("git"); + let mut command = codex_utils_process::background_command("git"); command.current_dir(dir); if let Some(envs) = env { for (key, value) in envs { diff --git a/codex-rs/hooks/Cargo.toml b/codex-rs/hooks/Cargo.toml index cb234830aa..4ad0b1ce97 100644 --- a/codex-rs/hooks/Cargo.toml +++ b/codex-rs/hooks/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } async-channel = { workspace = true } chrono = { workspace = true, features = ["serde"] } diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index d721500c17..674f5bd2ed 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -3,6 +3,8 @@ use std::ffi::OsStr; use std::ffi::OsString; use std::future::Future; use std::io::ErrorKind; +#[cfg(windows)] +use std::os::windows::process::CommandExt; use std::path::Path; #[cfg(not(unix))] use std::process::Stdio; @@ -27,6 +29,9 @@ use tokio::task::JoinSet; use tokio::time::timeout; use tracing::Span; +#[cfg(windows)] +const CREATE_NO_WINDOW: u32 = 0x08000000; + use super::CommandShell; use super::ConfiguredHandler; use super::ConfiguredHandlerKind; @@ -221,18 +226,9 @@ pub(crate) async fn run_command( command.current_dir(cwd); #[cfg(windows)] - let mut process_tree_job = JobObject::create().ok(); - #[cfg(windows)] - let child = match process_tree_job.as_ref() { - Some(job) => match job.spawn_contained(&mut command) { - Ok(child) => Ok(child), - Err(_) => { - process_tree_job = None; - command.creation_flags(0); - command.spawn() - } - }, - None => command.spawn(), + let (child, process_tree_job) = match JobObject::spawn_background(&mut command) { + Ok((child, job)) => (Ok(child), job), + Err(error) => (Err(error), None), }; #[cfg(not(windows))] let child = command.spawn(); @@ -353,6 +349,7 @@ impl Drop for ProcessTreeGuard { .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) + .creation_flags(CREATE_NO_WINDOW) .spawn(); } } @@ -414,6 +411,8 @@ fn build_command( #[cfg(unix)] command.process_mode(codex_utils_pty::ProcessMode::NewSession); + #[cfg(windows)] + command.creation_flags(CREATE_NO_WINDOW); #[cfg(not(unix))] command .env_clear() diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 5a033f9c64..2049ac6c95 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -327,7 +327,7 @@ pub(crate) fn command_from_argv( if program.is_empty() { return None; } - let mut command = Command::new(program); + let mut command = Command::from(codex_utils_process::background_command(program)); command.args(args); command.env_clear(); command.envs(environment); diff --git a/codex-rs/login/Cargo.toml b/codex-rs/login/Cargo.toml index 2971dec6c1..8772f46b58 100644 --- a/codex-rs/login/Cargo.toml +++ b/codex-rs/login/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true workspace = true [dependencies] +codex-utils-process = { workspace = true } base64 = { workspace = true } chrono = { workspace = true, features = ["serde"] } codex-agent-identity = { workspace = true } diff --git a/codex-rs/login/src/auth/external_bearer.rs b/codex-rs/login/src/auth/external_bearer.rs index 9f3fb64871..b8ad0d0672 100644 --- a/codex-rs/login/src/auth/external_bearer.rs +++ b/codex-rs/login/src/auth/external_bearer.rs @@ -102,7 +102,7 @@ struct CachedExternalBearerToken { async fn run_provider_auth_command(config: &ModelProviderAuthInfo) -> io::Result { let program = resolve_provider_auth_program(&config.command, &config.cwd)?; - let mut command = Command::new(&program); + let mut command = Command::from(codex_utils_process::background_command(&program)); command .args(config.args.iter().map(Deref::deref)) .current_dir(config.cwd.as_path()) diff --git a/codex-rs/model-provider/Cargo.toml b/codex-rs/model-provider/Cargo.toml index ecdcaa4b3e..26c67921cb 100644 --- a/codex-rs/model-provider/Cargo.toml +++ b/codex-rs/model-provider/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } chrono = { workspace = true } codex-api = { workspace = true } codex-agent-identity = { workspace = true } diff --git a/codex-rs/model-provider/src/amazon_bedrock/auth_refresh.rs b/codex-rs/model-provider/src/amazon_bedrock/auth_refresh.rs index d15b5673dd..a193243b23 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/auth_refresh.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/auth_refresh.rs @@ -43,10 +43,17 @@ impl AwsAuthRecovery { return Ok(()); } - let mut command = Command::new(&self.config.command); + let interactive = std::io::stdin().is_terminal(); + let mut command = if interactive { + Command::new(&self.config.command) + } else { + Command::from(codex_utils_process::background_command( + &self.config.command, + )) + }; command .args(self.config.args.iter().map(Deref::deref)) - .stdin(if std::io::stdin().is_terminal() { + .stdin(if interactive { Stdio::inherit() } else { Stdio::null() diff --git a/codex-rs/model-provider/src/amazon_bedrock/credential_export.rs b/codex-rs/model-provider/src/amazon_bedrock/credential_export.rs index 7cabae3376..d6db7d93ed 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/credential_export.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/credential_export.rs @@ -171,7 +171,7 @@ impl AwsCredentialExport { )); } - let mut command = Command::new(program); + let mut command = Command::from(codex_utils_process::background_command(program)); command .args(self.config.args.iter().map(Deref::deref)) .stdin(Stdio::null()) diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 13fabc46d9..433f532a13 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -214,11 +214,6 @@ impl StdioServerLauncher for LocalStdioServerLauncher { // Local private implementation. -#[cfg(windows)] -const CREATE_NO_WINDOW: u32 = 0x0800_0000; -#[cfg(windows)] -const CREATE_SUSPENDED: u32 = 0x0000_0004; - #[cfg(unix)] const PROCESS_GROUP_TERM_GRACE_PERIOD: Duration = Duration::from_secs(2); @@ -293,8 +288,6 @@ impl LocalStdioServerLauncher { // handles and needs a handle allowlist in the shared spawn backend. #[cfg(unix)] command.descriptor_policy(DescriptorPolicy::Explicit); - #[cfg(windows)] - command.creation_flags(CREATE_NO_WINDOW); command }; #[cfg(windows)] @@ -305,8 +298,6 @@ impl LocalStdioServerLauncher { let job = match codex_utils_pty::JobObject::create_without_breakaway() { Ok(job) => { command.prepare_suspended_spawn(&job); - // The helper replaces creation flags; retain suspension and suppress the console. - command.creation_flags(CREATE_SUSPENDED | CREATE_NO_WINDOW); Some(job) } Err(error) => { diff --git a/codex-rs/rollout/Cargo.toml b/codex-rs/rollout/Cargo.toml index 1bb4b54f14..8981dd9662 100644 --- a/codex-rs/rollout/Cargo.toml +++ b/codex-rs/rollout/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true, features = ["serde"] } codex-extension-items = { workspace = true } diff --git a/codex-rs/rollout/src/search.rs b/codex-rs/rollout/src/search.rs index 1f2a132001..b9c2bc7343 100644 --- a/codex-rs/rollout/src/search.rs +++ b/codex-rs/rollout/src/search.rs @@ -71,7 +71,7 @@ async fn ripgrep_rollout_paths( return Ok(Some(HashSet::new())); } - let output = match Command::new(rg_command) + let output = match Command::from(codex_utils_process::background_command(rg_command)) .arg("-l") .arg("--fixed-strings") .arg("--ignore-case") diff --git a/codex-rs/utils/process/BUILD.bazel b/codex-rs/utils/process/BUILD.bazel new file mode 100644 index 0000000000..c089719d4e --- /dev/null +++ b/codex-rs/utils/process/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "process", + crate_name = "codex_utils_process", +) diff --git a/codex-rs/utils/process/Cargo.toml b/codex-rs/utils/process/Cargo.toml new file mode 100644 index 0000000000..3d5bbd66ad --- /dev/null +++ b/codex-rs/utils/process/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "codex-utils-process" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dev-dependencies] +pretty_assertions = { workspace = true } +tokio = { workspace = true, features = ["macros", "process", "rt-multi-thread"] } + +[lib] +doctest = false diff --git a/codex-rs/utils/process/src/background_command_tests.rs b/codex-rs/utils/process/src/background_command_tests.rs new file mode 100644 index 0000000000..216b3279c2 --- /dev/null +++ b/codex-rs/utils/process/src/background_command_tests.rs @@ -0,0 +1,54 @@ +//! Verifies detached background children stay console-free through std and Tokio launches. + +use super::background_command; +use pretty_assertions::assert_eq; +use std::os::windows::process::CommandExt; +use std::process::Command; + +#[tokio::test] +async fn background_commands_have_no_console_under_detached_parent() { + const TEST_NAME: &str = "tests::background_commands_have_no_console_under_detached_parent"; + const PHASE: &str = "CODEX_TEST_BACKGROUND_COMMAND_PHASE"; + let executable = std::env::current_exe().expect("test executable"); + match std::env::var(PHASE).as_deref() { + Ok("probe") => { + #[link(name = "kernel32")] + unsafe extern "system" { + #[link_name = "GetConsoleWindow"] + fn get_console_window() -> isize; + } + assert_eq!(unsafe { get_console_window() }, 0); + println!("background stdout"); + eprintln!("background stderr"); + } + Ok("detached") => { + for asynchronous in [false, true] { + let mut command = background_command(&executable); + command + .args(["--exact", TEST_NAME, "--nocapture"]) + .env(PHASE, "probe"); + let output = if asynchronous { + tokio::process::Command::from(command).output().await + } else { + command.output() + } + .expect("run background command"); + assert!(output.status.success(), "probe failed: {output:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("background stdout")); + assert!(String::from_utf8_lossy(&output.stderr).contains("background stderr")); + } + } + _ => { + let output = Command::new(executable) + .args(["--exact", TEST_NAME, "--nocapture"]) + .env(PHASE, "detached") + .creation_flags(/*flags*/ 0x0000_0008) // DETACHED_PROCESS + .output() + .expect("spawn detached parent"); + assert!( + output.status.success(), + "detached parent failed: {output:?}" + ); + } + } +} diff --git a/codex-rs/utils/process/src/lib.rs b/codex-rs/utils/process/src/lib.rs new file mode 100644 index 0000000000..b3c4491821 --- /dev/null +++ b/codex-rs/utils/process/src/lib.rs @@ -0,0 +1,26 @@ +//! Command construction for background processes that must not allocate a Windows console. +//! Callers retain control of environment, stdio, and process lifetime. Interactive, +//! detached, and private-desktop launches must select their own console policy. + +use std::ffi::OsStr; +use std::process::Command; + +/// Construct a background command, suppressing console allocation on Windows. +/// Convert with `tokio::process::Command::from` for asynchronous execution. +/// Subsequent Windows `creation_flags` calls must retain `CREATE_NO_WINDOW`. +pub fn background_command(program: impl AsRef) -> Command { + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + + let mut command = Command::new(program); + command.creation_flags(/*flags*/ 0x0800_0000); // CREATE_NO_WINDOW + command + } + #[cfg(not(windows))] + Command::new(program) +} + +#[cfg(all(test, windows))] +#[path = "background_command_tests.rs"] +mod tests; diff --git a/codex-rs/utils/pty/src/child_command.rs b/codex-rs/utils/pty/src/child_command.rs index 26d053acca..452e3ecdde 100644 --- a/codex-rs/utils/pty/src/child_command.rs +++ b/codex-rs/utils/pty/src/child_command.rs @@ -15,8 +15,6 @@ use std::path::Path; use std::process::Stdio as TokioStdio; #[cfg(windows)] use winapi::um::winbase::CREATE_NO_WINDOW; -#[cfg(windows)] -use winapi::um::winbase::CREATE_SUSPENDED; use crate::child::Child; use crate::child::ChildKind; @@ -240,19 +238,10 @@ impl Command { self } - /// Set the Windows process creation flags, replacing any previously selected flags. - #[cfg(windows)] - pub fn creation_flags(&mut self, flags: u32) -> &mut Self { - self.inner.creation_flags(flags); - self - } /// Preserve Job Object assignment before the child begins executing on Windows. #[cfg(windows)] pub fn prepare_suspended_spawn(&mut self, job: &crate::JobObject) { job.prepare_suspended_spawn(&mut self.inner); - // Tokio's creation_flags replaces, rather than adds to, the flags. - self.inner - .creation_flags(CREATE_NO_WINDOW | CREATE_SUSPENDED); } /// Reject original inputs that std replaced with a NUL-free placeholder. diff --git a/codex-rs/utils/pty/src/win/job.rs b/codex-rs/utils/pty/src/win/job.rs index b06035f72d..f963c6f55f 100644 --- a/codex-rs/utils/pty/src/win/job.rs +++ b/codex-rs/utils/pty/src/win/job.rs @@ -14,6 +14,7 @@ use winapi::um::jobapi2::SetInformationJobObject; use winapi::um::jobapi2::TerminateJobObject; use winapi::um::processthreadsapi::OpenProcess; use winapi::um::processthreadsapi::TerminateProcess; +use winapi::um::winbase::CREATE_NO_WINDOW; use winapi::um::winbase::CREATE_SUSPENDED; use winapi::um::winnt::HANDLE; use winapi::um::winnt::JOB_OBJECT_LIMIT_BREAKAWAY_OK; @@ -121,9 +122,14 @@ impl JobObject { } } - /// Prevents a child from running before it can be assigned to this job. + /// Prepares a background child to be assigned to this job before it runs. + /// + /// Replaces all creation flags with `CREATE_SUSPENDED | CREATE_NO_WINDOW`. + /// Interactive children that need to inherit a console require an explicit launch. pub fn prepare_suspended_spawn(&self, command: &mut Command) { - command.creation_flags(CREATE_SUSPENDED).kill_on_drop(true); + command + .creation_flags(CREATE_NO_WINDOW | CREATE_SUSPENDED) + .kill_on_drop(true); } /// Assigns and resumes a suspended child, returning whether assignment succeeded. @@ -165,7 +171,29 @@ impl JobObject { } } - /// Starts a process only after assigning it to this Job Object. + /// Starts a console-free background process, falling back when containment is unavailable. + /// The returned job owns the contained process tree when assignment succeeds. + pub fn spawn_background(command: &mut Command) -> io::Result<(Child, Option)> { + Self::spawn_background_with_job(command, Self::create()) + } + + fn spawn_background_with_job( + command: &mut Command, + job: io::Result, + ) -> io::Result<(Child, Option)> { + match job.and_then(|job| job.spawn_contained(command).map(|child| (child, job))) { + Ok((child, job)) => Ok((child, Some(job))), + Err(_) => { + // Remove suspension before retrying, including after failed assignment. + command.creation_flags(CREATE_NO_WINDOW).kill_on_drop(true); + command.spawn().map(|child| (child, None)) + } + } + } + + /// Starts a background child without a console, assigning it to this job before it runs. + /// + /// Replaces existing creation flags as in [`Self::prepare_suspended_spawn`]. pub fn spawn_contained(&self, command: &mut Command) -> io::Result { self.prepare_suspended_spawn(command); let child = command.spawn()?; @@ -230,3 +258,7 @@ impl AsRawHandle for JobObject { self.handle.as_raw_handle() } } + +#[cfg(test)] +#[path = "job_tests.rs"] +mod tests; diff --git a/codex-rs/utils/pty/src/win/job_tests.rs b/codex-rs/utils/pty/src/win/job_tests.rs new file mode 100644 index 0000000000..3d430d363c --- /dev/null +++ b/codex-rs/utils/pty/src/win/job_tests.rs @@ -0,0 +1,87 @@ +//! Verifies console suppression through contained launches and both containment fallbacks. + +use super::*; +use pretty_assertions::assert_eq; +use std::os::windows::process::CommandExt; +use std::process::Stdio; +use std::time::Duration; +use winapi::um::winnt::JOB_OBJECT_LIMIT_ACTIVE_PROCESS; + +#[tokio::test] +async fn background_launches_keep_consoles_hidden_when_containment_fails() -> anyhow::Result<()> { + const TEST_NAME: &str = + "win::job::tests::background_launches_keep_consoles_hidden_when_containment_fails"; + const PHASE: &str = "CODEX_TEST_BACKGROUND_JOB_PHASE"; + let executable = std::env::current_exe()?; + match std::env::var(PHASE).as_deref() { + Ok("probe") => { + assert!(unsafe { winapi::um::wincon::GetConsoleWindow() }.is_null()); + println!("background probe completed"); + return Ok(()); + } + Ok("hold") => { + std::thread::sleep(Duration::from_secs(/*secs*/ 30)); + return Ok(()); + } + Ok("parent") => {} + _ => { + let output = std::process::Command::new(executable) + .args(["--exact", TEST_NAME, "--nocapture"]) + .env(PHASE, "parent") + .creation_flags(winapi::um::winbase::DETACHED_PROCESS) + .output()?; + assert!(output.status.success(), "detached probe failed: {output:?}"); + return Ok(()); + } + } + + // Fill a one-process job so assigning the next suspended child must fail. + // The parent remains outside this job, allowing the uncontained retry to run. + let full_job = JobObject::create()?; + let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() }; + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_ACTIVE_PROCESS + | JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + | JOB_OBJECT_LIMIT_BREAKAWAY_OK; + limits.BasicLimitInformation.ActiveProcessLimit = 1; + let configured = unsafe { + SetInformationJobObject( + full_job.handle.as_raw_handle().cast(), + JobObjectExtendedLimitInformation, + std::ptr::addr_of_mut!(limits).cast(), + std::mem::size_of::() as u32, + ) + }; + anyhow::ensure!(configured != 0, "{}", io::Error::last_os_error()); + let mut holder_command = Command::new(&executable); + holder_command + .args(["--exact", TEST_NAME]) + .env(PHASE, "hold"); + let mut holder = full_job.spawn_contained(&mut holder_command)?; + + for (case, job, expected_containment) in [ + ( + "creation failure", + Err(io::Error::other("forced creation failure")), + false, + ), + ("assignment failure", Ok(full_job), false), + ("contained", JobObject::create(), true), + ] { + let mut command = Command::new(&executable); + command + .args(["--exact", TEST_NAME, "--nocapture"]) + .env(PHASE, "probe") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let (child, job) = JobObject::spawn_background_with_job(&mut command, job)?; + assert_eq!(job.is_some(), expected_containment, "{case}"); + let output = + tokio::time::timeout(Duration::from_secs(/*secs*/ 10), child.wait_with_output()) + .await??; + assert!(output.status.success(), "{case}: {output:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("background probe completed")); + } + tokio::time::timeout(Duration::from_secs(/*secs*/ 10), holder.wait()).await??; + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/Cargo.toml b/codex-rs/windows-sandbox-rs/Cargo.toml index b182ed8c9e..65332f3506 100644 --- a/codex-rs/windows-sandbox-rs/Cargo.toml +++ b/codex-rs/windows-sandbox-rs/Cargo.toml @@ -26,6 +26,7 @@ path = "src/bin/managed_deny_probe/main.rs" workspace = true [dependencies] +codex-utils-process = { workspace = true } codex-http-client = { workspace = true } anyhow = "1.0" base64 = { workspace = true } diff --git a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win/cwd_junction.rs b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win/cwd_junction.rs index 7ddd391c34..9243cc67aa 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win/cwd_junction.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win/cwd_junction.rs @@ -100,7 +100,7 @@ pub fn create_cwd_junction(requested_cwd: &Path, log_dir: Option<&Path>) -> Opti &format!("junction: creating via cmd /c mklink /J {link_quoted} {target_quoted}"), log_dir, ); - let output = match std::process::Command::new("cmd") + let output = match codex_utils_process::background_command("cmd") .raw_arg("/c") .raw_arg("mklink") .raw_arg("/J") diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs index f7b39839f9..ed11fb14a3 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs @@ -7,7 +7,6 @@ use codex_protocol::permissions::windows_deny_read_glob_scan; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashSet; use std::path::PathBuf; -use std::process::Command; #[path = "deny_read_walker.rs"] mod walker; @@ -100,7 +99,7 @@ pub fn resolve_windows_deny_read_paths( } fn ripgrep_files(scan_plan: &GlobScanPlan) -> Result>, String> { - let mut command = Command::new("rg"); + let mut command = codex_utils_process::background_command("rg"); command .arg("--files") .arg("--hidden") diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 3c9e84de9d..26c40ec680 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -764,7 +764,7 @@ mod windows_impl { &env_map, logs_base_dir, Some((in_r, out_w, err_w)), - ConsoleMode::Inherit, + ConsoleMode::NoWindow, desktop, ) }); diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index c40a56c996..484f286713 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -386,7 +386,7 @@ fn run_setup_refresh_payload(b64: &str, codex_home: &Path) -> Result<()> { } }; // Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC. - let mut cmd = Command::new(&exe); + let mut cmd = codex_utils_process::background_command(&exe); crate::launch_environment::configure_command(&mut cmd, b64)?; cmd.stdin(Stdio::null()) .stdout(Stdio::null()) @@ -926,6 +926,10 @@ fn verify_setup_completed(codex_home: &Path) -> Result<()> { } } +#[cfg(test)] +#[path = "setup_refresh_tests.rs"] +mod refresh_tests; + fn run_setup_exe( payload: &ElevationPayload, needs_elevation: bool, diff --git a/codex-rs/windows-sandbox-rs/src/setup_refresh_tests.rs b/codex-rs/windows-sandbox-rs/src/setup_refresh_tests.rs new file mode 100644 index 0000000000..0a9f5d5315 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/setup_refresh_tests.rs @@ -0,0 +1,64 @@ +//! Verifies refresh helpers stay console-free when launched by a detached daemon. + +use std::fs; +use std::os::windows::process::CommandExt; +use std::path::PathBuf; +use std::process::Command; + +use anyhow::Result; +use pretty_assertions::assert_eq; +use windows_sys::Win32::System::Console::GetConsoleWindow; +use windows_sys::Win32::System::Threading::DETACHED_PROCESS; + +use super::run_setup_refresh_payload; + +const DETACHED_TEST: &str = "setup::refresh_tests::refresh_helper_has_no_console"; +const HELPER_TEST: &str = "setup::refresh_tests::refresh_console_probe"; +const HOME_ENV: &str = "CODEX_TEST_REFRESH_CONSOLE_HOME"; + +#[test] +fn refresh_helper_has_no_console() -> Result<()> { + if let Some(home) = std::env::var_os(HOME_ENV).map(PathBuf::from) { + assert_eq!(unsafe { GetConsoleWindow() }, 0); + run_setup_refresh_payload(HELPER_TEST, &home)?; + assert_eq!(fs::read(home.join("probe"))?, b"no console"); + return Ok(()); + } + + // Re-execute detached with a disposable fake setup helper beside it. This + // exercises the production refresh launch without provisioning sandbox users. + let temporary = tempfile::tempdir()?; + let executable = std::env::current_exe()?; + let detached = temporary.path().join("detached.exe"); + fs::copy(&executable, &detached)?; + fs::copy( + &executable, + temporary.path().join("codex-windows-sandbox-setup.exe"), + )?; + let output = Command::new(detached) + .args(["--exact", DETACHED_TEST, "--nocapture"]) + .env(HOME_ENV, temporary.path()) + .creation_flags(DETACHED_PROCESS) + .output()?; + assert!( + output.status.success(), + "detached refresh probe failed:\n{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + assert_eq!(fs::read(temporary.path().join("probe"))?, b"no console"); + Ok(()) +} + +#[test] +fn refresh_console_probe() -> Result<()> { + // The setup launcher supplies this test name as its single payload argument. + // Ordinary test runs must not require their own parent to be console-free. + if std::env::args().nth(1).as_deref() != Some(HELPER_TEST) { + return Ok(()); + } + let home = PathBuf::from(std::env::var_os(HOME_ENV).expect("detached probe home")); + assert_eq!(unsafe { GetConsoleWindow() }, 0); + fs::write(home.join("probe"), b"no console")?; + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index 62a82a7b9f..f6726d7275 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -115,7 +115,7 @@ fn spawn_legacy_process( StdinMode::Closed }, StderrMode::Separate, - ConsoleMode::Inherit, + ConsoleMode::NoWindow, launch_desktop, logs_base_dir, )?; diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs index 1bdf4f8136..f35ee56b38 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs @@ -50,6 +50,8 @@ use windows_sys::Win32::System::Threading::WaitForSingleObject; static TEST_HOME_COUNTER: AtomicU64 = AtomicU64::new(0); static LEGACY_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(()); +const ASSERT_NO_CONSOLE: &str = r#"Add-Type -ErrorAction Stop 'using System; using System.Runtime.InteropServices; public class ConsoleProbe { [DllImport("kernel32.dll")] public static extern IntPtr GetConsoleWindow(); }'; if ([ConsoleProbe]::GetConsoleWindow() -ne [IntPtr]::Zero) { throw 'piped sandbox process unexpectedly has a console' };"#; + fn legacy_process_test_guard() -> MutexGuard<'static, ()> { LEGACY_PROCESS_TEST_LOCK .lock() @@ -460,7 +462,7 @@ fn legacy_non_tty_powershell_interrupt_terminates_process() { pwsh.display().to_string(), "-NoProfile".to_string(), "-Command".to_string(), - "Write-Output LEGACY-NONTTY-DIRECT; [System.Threading.ManualResetEvent]::new($false).WaitOne()".to_string(), + format!("{ASSERT_NO_CONSOLE} Write-Output LEGACY-NONTTY-DIRECT; [System.Threading.ManualResetEvent]::new($false).WaitOne()"), ], cwd.as_path(), HashMap::new(), @@ -681,7 +683,7 @@ fn legacy_capture_emits_output_and_preserves_descendant_after_normal_exit() { powershell_literal(&ready_marker), ); let parent_command = format!( - "Write-Output LEGACY-CAPTURE-DIRECT; {}", + "{ASSERT_NO_CONSOLE} Write-Output LEGACY-CAPTURE-DIRECT; {}", start_powershell_child(&pwsh, codex_home.path(), &descendant_command, &parent_tail,), ); let permission_profile = PermissionProfile::workspace_write();