mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Launch Windows daemon children in a dedicated working directory (#49850)
## Why Windows processes hold their working directory open for their lifetime, and sandbox setup may broaden its ACL. Managed children should avoid pinning the launching project's directory or changing permissions on the private daemon state directory. ## What changed Create a `workdir` subdirectory alongside the PID files and use it as the working directory for both the managed app-server and updater on Windows. ## Testing Add a Windows regression test covering both managed children. It verifies that relative files land in `workdir`, the state directory still passes private-directory validation, and stopping each child removes its PID file. GitOrigin-RevId: 12ca95463ff3c80899a5b1a8800fead849f48017
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
//! Detached process launch and PID publication. Hold the reservation lock until
|
||||
//! the record is published, and on Windows until an updater acknowledges startup.
|
||||
//! Windows children use a separate working directory to preserve the state directory ACL.
|
||||
//! Recover a deleted Unix cwd without changing workspace defaults for usable directories.
|
||||
|
||||
use super::PidBackend;
|
||||
@@ -25,6 +26,21 @@ impl PidBackend {
|
||||
.await
|
||||
.with_context(|| format!("failed to create pid directory {}", parent.display()))?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
let workdir = {
|
||||
let workdir = self
|
||||
.pid_file
|
||||
.parent()
|
||||
.context("daemon pid path has no parent")?
|
||||
.join("workdir");
|
||||
fs::create_dir_all(&workdir).await.with_context(|| {
|
||||
format!(
|
||||
"failed to create daemon working directory {}",
|
||||
workdir.display()
|
||||
)
|
||||
})?;
|
||||
workdir
|
||||
};
|
||||
let reservation_lock = self.acquire_reservation_lock().await?;
|
||||
loop {
|
||||
match fs::OpenOptions::new()
|
||||
@@ -157,6 +173,10 @@ impl PidBackend {
|
||||
{
|
||||
use windows_sys::Win32::System::Threading::CREATE_BREAKAWAY_FROM_JOB;
|
||||
use windows_sys::Win32::System::Threading::DETACHED_PROCESS;
|
||||
// A Windows process pins its working directory for its lifetime.
|
||||
// Keep both managed children out of the launching project's directory.
|
||||
// Sandbox setup may broaden the cwd ACL, so do not use the private state directory.
|
||||
command.current_dir(&workdir);
|
||||
// Never retry inside the parent's Job Object: that would report a
|
||||
// successful launch that dies when the terminal/SSH session closes.
|
||||
command.creation_flags(DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB);
|
||||
|
||||
@@ -705,6 +705,61 @@ async fn stale_creation_time_never_stops_reused_pid() {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn managed_children_launch_in_workdir_without_changing_private_state_directory() {
|
||||
if is_elevated_test_process().expect("query administrator membership") {
|
||||
return;
|
||||
}
|
||||
let temp = TempDir::new().expect("temp dir");
|
||||
let state_dir = temp.path().join("state");
|
||||
let codex_bin = temp.path().join("codex.cmd");
|
||||
tokio::fs::write(
|
||||
&codex_bin,
|
||||
"@echo off\r\n\
|
||||
if \"%3\"==\"--help\" exit /b 1\r\n\
|
||||
echo ready > launched.cwd\r\n\
|
||||
if defined CODEX_DAEMON_SHUTDOWN_FILE (for %%F in (\"%CODEX_DAEMON_SHUTDOWN_FILE%\") do type nul > \"%%~dpnF.ready\")\r\n\
|
||||
for /l %%i in (1,1,10000000) do @rem\r\n",
|
||||
)
|
||||
.await
|
||||
.expect("write daemon fixture");
|
||||
let backends = [
|
||||
PidBackend::new(
|
||||
codex_bin.clone(),
|
||||
state_dir.join("app-server.pid"),
|
||||
/*remote_control_enabled*/ false,
|
||||
),
|
||||
PidBackend::new_update_loop(
|
||||
codex_bin,
|
||||
state_dir.join("updater.pid"),
|
||||
/*restore_release*/ None,
|
||||
),
|
||||
];
|
||||
for backend in backends {
|
||||
backend.start().await.expect("launch managed child");
|
||||
let marker = state_dir.join("workdir/launched.cwd");
|
||||
let launched_in_workdir = tokio::time::timeout(Duration::from_secs(5), async {
|
||||
while !marker.exists() {
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await;
|
||||
backend
|
||||
.stop_with_grace(/*grace_seconds*/ 0)
|
||||
.await
|
||||
.expect("stop managed child");
|
||||
launched_in_workdir.expect("child did not write its relative marker in workdir");
|
||||
codex_uds::prepare_private_socket_directory(&state_dir)
|
||||
.await
|
||||
.expect("state directory remains private");
|
||||
assert!(!backend.pid_file.exists());
|
||||
tokio::fs::remove_file(marker)
|
||||
.await
|
||||
.expect("remove marker before next child launch");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn failed_updater_handoff_preserves_predecessor_record() {
|
||||
|
||||
Reference in New Issue
Block a user