mirror of
https://github.com/openai/codex.git
synced 2026-10-16 16:58:46 +00:00
Suppress Windows console windows for background subprocesses (#49164)
## Why Background helpers launched from a detached Windows process can allocate console windows. Job Object launches and containment fallbacks need to preserve console suppression as well. ## What changed - Add `codex_utils_process::background_command` to set `CREATE_NO_WINDOW` on Windows, and use it across background subprocess launches, including Git, plugins, authentication, diagnostics, and sandbox helpers. - Centralize console suppression in Job Object launches. Keep `CREATE_NO_WINDOW` when retrying without containment, and clear `CREATE_SUSPENDED` so the fallback child can run. - Preserve console inheritance for interactive shell launches and interactive AWS authentication refreshes. ## Testing Add Windows regression tests covering console-free launches from detached parents through standard and Tokio commands, stdout/stderr capture, successful containment, Job Object creation and assignment failures, and sandbox setup refresh helpers. GitOrigin-RevId: b2c238fa7d4f9fbd868e5adf7d4be9958ff01e6c
This commit is contained in:
Generated
+19
@@ -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",
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"] }
|
||||
|
||||
@@ -72,7 +72,7 @@ async fn query_channel(wevtutil: &Path, channel: Channel) -> Option<Vec<Evidence
|
||||
.map(|id| format!("EventID={id}"))
|
||||
.collect::<Vec<_>>()
|
||||
.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]]]"
|
||||
|
||||
@@ -236,7 +236,7 @@ async fn endpoint_products() -> EndpointInspection {
|
||||
|
||||
#[cfg(any(target_os = "macos", target_os = "windows"))]
|
||||
async fn product_command(program: impl AsRef<OsStr>, args: &[&str]) -> Option<Output> {
|
||||
let mut command = Command::new(program);
|
||||
let mut command = Command::from(codex_utils_process::background_command(program));
|
||||
command
|
||||
.args(args)
|
||||
.stdin(Stdio::null())
|
||||
|
||||
@@ -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(());
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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()?;
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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"] }
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -102,7 +102,7 @@ struct CachedExternalBearerToken {
|
||||
|
||||
async fn run_provider_auth_command(config: &ModelProviderAuthInfo) -> io::Result<String> {
|
||||
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())
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "process",
|
||||
crate_name = "codex_utils_process",
|
||||
)
|
||||
@@ -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
|
||||
@@ -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:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<OsStr>) -> 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;
|
||||
@@ -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.
|
||||
|
||||
@@ -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>)> {
|
||||
Self::spawn_background_with_job(command, Self::create())
|
||||
}
|
||||
|
||||
fn spawn_background_with_job(
|
||||
command: &mut Command,
|
||||
job: io::Result<Self>,
|
||||
) -> io::Result<(Child, Option<Self>)> {
|
||||
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<Child> {
|
||||
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;
|
||||
|
||||
@@ -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::<JOBOBJECT_EXTENDED_LIMIT_INFORMATION>() 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(())
|
||||
}
|
||||
@@ -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 }
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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<Option<Vec<PathBuf>>, String> {
|
||||
let mut command = Command::new("rg");
|
||||
let mut command = codex_utils_process::background_command("rg");
|
||||
command
|
||||
.arg("--files")
|
||||
.arg("--hidden")
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
Reference in New Issue
Block a user