From b989f795b11ad4897b6acb7c1b9b5cc202782fd6 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 4 Oct 2026 06:18:48 +0000 Subject: [PATCH] Fall back to mklink when Windows daemon junction updates are denied (#50802) ## Why Some Windows policies deny in-process reparse-point mutation while allowing the system junction creator, preventing managed daemon release selection. ## What changed Fall back to `cmd.exe` with `mklink /J` when native junction creation or retargeting returns `PermissionDenied`. Pass quoted paths through environment variables and disable delayed expansion to handle shell metacharacters. Create the replacement junction before moving the existing `current` junction aside, and attempt to restore the previous junction if installation fails. Preserve it for recovery if restoration also fails. ## Testing Extend the Windows junction test to cover fallback creation, native retargeting, and fallback replacement under a path containing spaces, `&`, `%`, and `!`, while verifying that no standalone CLI installation is created. GitOrigin-RevId: 039ee850f8c788f44f08116614ca3927e3e9952f --- .../src/prepare_install_windows.rs | 105 ++++++++++++++++-- .../src/prepare_install_windows_tests.rs | 22 +++- 2 files changed, 111 insertions(+), 16 deletions(-) diff --git a/codex-rs/app-server-daemon/src/prepare_install_windows.rs b/codex-rs/app-server-daemon/src/prepare_install_windows.rs index 0467fa0740..953f6ddea9 100644 --- a/codex-rs/app-server-daemon/src/prepare_install_windows.rs +++ b/codex-rs/app-server-daemon/src/prepare_install_windows.rs @@ -3,7 +3,9 @@ use std::os::windows::ffi::OsStrExt; use std::os::windows::fs::OpenOptionsExt; use std::os::windows::io::AsRawHandle; +use std::os::windows::process::CommandExt; use std::path::Path; +use std::process::Command; use std::time::Duration; use anyhow::Context; @@ -15,6 +17,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::System::IO::DeviceIoControl; +use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW; const PUBLISH_RELEASE_RETRY_INTERVAL: Duration = Duration::from_millis(50); const PUBLISH_RELEASE_RETRY_LIMIT: usize = 100; @@ -47,30 +50,97 @@ pub(super) async fn publish_release(stage: &Path, release: &Path) -> Result<()> } pub(super) fn select_release(root: &Path, release: &Path) -> Result<()> { + select_release_with(root, release, retarget_junction) +} + +fn select_release_with( + root: &Path, + release: &Path, + retarget: impl FnOnce(&Path, &Path) -> Result<()>, +) -> Result<()> { let release = release.canonicalize()?; let current = root.join("current"); if current.symlink_metadata().is_err() { let temporary = tempfile::TempDir::new_in(root)?; let junction = temporary.path().join("current"); std::fs::create_dir(&junction)?; - retarget_junction(&junction, &release)?; - std::fs::rename(junction, ¤t)?; - return Ok(()); + return match retarget(&junction, &release) { + Ok(()) => Ok(std::fs::rename(junction, current)?), + Err(error) if is_permission_denied(&error) => install_junction(root, &release) + .with_context(|| format!("failed to create managed daemon junction after native creation was denied: {error:#}")), + Err(error) => Err(error), + }; } validate_selection(root)?; - retarget_junction(¤t, &release) + match retarget(¤t, &release) { + Ok(()) => Ok(()), + Err(error) if is_permission_denied(&error) => { + install_junction(root, &release).with_context(|| format!("failed to replace managed daemon junction after retargeting was denied: {error:#}")) + } + Err(error) => Err(error), + } } -pub(super) fn validate_selection(root: &Path) -> Result<()> { +fn is_permission_denied(error: &anyhow::Error) -> bool { + error + .downcast_ref::() + .is_some_and(|error| error.kind() == std::io::ErrorKind::PermissionDenied) +} + +fn install_junction(root: &Path, release: &Path) -> Result<()> { + let temporary = tempfile::TempDir::new_in(root)?; + let junction = temporary.path().join("current"); let current = root.join("current"); - if matches!(current.symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound) - { + let replacing = current.symlink_metadata().is_ok(); + if replacing { + validate_selection(root)?; + } + // Some Windows policies deny in-process reparse-point mutation while allowing the system + // junction creator. Pass paths through the environment so cmd metacharacters stay quoted. + let system_root = std::env::var_os("SystemRoot").context("SystemRoot is not set")?; + let command_shell = Path::new(&system_root).join("System32").join("cmd.exe"); + anyhow::ensure!( + command_shell.is_absolute(), + "SystemRoot must be an absolute path" + ); + let output = Command::new(command_shell) + .env("CODEX_DAEMON_JUNCTION_LINK", &junction) + .env("CODEX_DAEMON_JUNCTION_TARGET", release) + .args(["/d", "/s", "/e:on", "/v:off", "/c"]) + .raw_arg(r#""mklink /J "%CODEX_DAEMON_JUNCTION_LINK%" "%CODEX_DAEMON_JUNCTION_TARGET%"""#) + .creation_flags(CREATE_NO_WINDOW) + .output() + .context("failed to run cmd.exe to create managed daemon junction")?; + anyhow::ensure!( + output.status.success(), + "cmd.exe could not create managed daemon junction (status {}): stdout: {}; stderr: {}", + output.status, + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim() + ); + if !replacing { + std::fs::rename(junction, current)?; return Ok(()); } - anyhow::ensure!( - current.canonicalize()?.parent() == Some(root.join("releases").canonicalize()?.as_path()), - "refusing to replace a daemon selection outside its releases directory" - ); + + let previous = temporary.path().join("previous"); + std::fs::rename(¤t, &previous)?; + if let Err(error) = std::fs::rename(&junction, ¤t) { + if let Err(restore_error) = std::fs::rename(&previous, ¤t) { + let preserved = temporary.keep(); + anyhow::bail!( + "failed to replace managed daemon junction: {error}; also failed to restore the previous junction from {}: {restore_error}", + preserved.display() + ); + } + return Err(error).context("failed to replace managed daemon junction"); + } + if let Err(error) = std::fs::remove_dir(previous) { + tracing::warn!( + %error, + "failed to remove previous managed daemon junction" + ); + } Ok(()) } @@ -124,6 +194,19 @@ fn retarget_junction(current: &Path, release: &Path) -> Result<()> { Ok(()) } +pub(super) fn validate_selection(root: &Path) -> Result<()> { + let current = root.join("current"); + if matches!(current.symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound) + { + return Ok(()); + } + anyhow::ensure!( + current.canonicalize()?.parent() == Some(root.join("releases").canonicalize()?.as_path()), + "refusing to replace a daemon selection outside its releases directory" + ); + Ok(()) +} + #[cfg(test)] #[path = "prepare_install_windows_tests.rs"] mod tests; diff --git a/codex-rs/app-server-daemon/src/prepare_install_windows_tests.rs b/codex-rs/app-server-daemon/src/prepare_install_windows_tests.rs index cca156e459..e84e64d97d 100644 --- a/codex-rs/app-server-daemon/src/prepare_install_windows_tests.rs +++ b/codex-rs/app-server-daemon/src/prepare_install_windows_tests.rs @@ -1,4 +1,6 @@ //! Verifies Windows package publication and selection without requiring an elevated daemon. + +use anyhow::Context; use pretty_assertions::assert_eq; use std::os::windows::fs::OpenOptionsExt; use std::time::Duration; @@ -32,17 +34,27 @@ async fn publishing_release_retries_transient_access_denied() { #[cfg(windows)] #[test] -fn daemon_junction_can_be_created_and_retargeted_without_cli_links() { +fn daemon_junction_can_be_created_retargeted_and_replaced_without_cli_links() { let home = tempfile::TempDir::new().unwrap(); - let root = home.path().join("packages/app-server-daemon"); - for version in ["first", "second"] { + let root = home + .path() + .join("home & 100% ready!/packages/app-server-daemon"); + for (version, native_denied) in [("first", true), ("second", false), ("third", true)] { let release = root.join("releases").join(version); std::fs::create_dir_all(&release).unwrap(); - super::select_release(&root, &release).unwrap(); + if native_denied { + super::select_release_with(&root, &release, |_, _| { + Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied)) + .context("failed to retarget managed daemon junction") + }) + .unwrap(); + } else { + super::select_release(&root, &release).unwrap(); + } assert_eq!( root.join("current").canonicalize().unwrap(), release.canonicalize().unwrap() ); } - assert!(!home.path().join("packages/standalone").exists()); + assert!(!root.with_file_name("standalone").exists()); }