diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs index 34ffcfc129..dc84430ca5 100644 --- a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs @@ -43,22 +43,13 @@ impl WindowsSandboxRequestProcessor { ) -> Result<(), JSONRPCErrorError> { // Validate requirements before acknowledging setup so callers do not get a // `started` response for a Windows sandbox mode that cannot be persisted. - let command_cwd = params - .cwd - .map(PathBuf::from) - .unwrap_or_else(|| self.config.cwd.to_path_buf()); - let config = self - .config_manager - .load_for_cwd( - /*request_overrides*/ None, - ConfigOverrides { - cwd: Some(command_cwd.clone()), - ..Default::default() - }, - Some(command_cwd.clone()), - ) - .await - .map_err(|err| config_load_error(&err))?; + let (config, command_cwd) = load_setup_config( + &self.config_manager, + self.config.cwd.as_path(), + params.cwd.map(PathBuf::from), + ) + .await + .map_err(|err| config_load_error(&err))?; let setup_mode = resolve_allowed_windows_sandbox_setup_mode( config.config_layer_stack.requirements(), params.mode, @@ -181,6 +172,29 @@ impl WindowsSandboxRequestProcessor { } } +async fn load_setup_config( + manager: &ConfigManager, + fallback_cwd: &std::path::Path, + requested_cwd: Option, +) -> std::io::Result<(Config, PathBuf)> { + let cwd = requested_cwd.unwrap_or_else(|| fallback_cwd.to_path_buf()); + let config = manager + .load_for_cwd( + /*request_overrides*/ None, + ConfigOverrides { + cwd: Some(cwd.clone()), + ..Default::default() + }, + Some(cwd.clone()), + ) + .await?; + Ok((config, cwd)) +} + +#[cfg(test)] +#[path = "windows_sandbox_setup_config_tests.rs"] +mod setup_config_tests; + /// Resolves the requested API mode after checking that managed requirements allow it. fn resolve_allowed_windows_sandbox_setup_mode( requirements: &codex_config::ConfigRequirements, diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs new file mode 100644 index 0000000000..a9683e6341 --- /dev/null +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs @@ -0,0 +1,23 @@ +//! Exercise the setup RPC's real configuration loading without changing Windows accounts or ACLs. + +use super::ConfigManager; +use super::load_setup_config; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn explicit_cwd_remains_the_setup_workspace() -> anyhow::Result<()> { + let home = tempfile::tempdir()?; + let install = tempfile::tempdir()?; + let project = tempfile::tempdir()?; + let manager = ConfigManager::without_managed_config_for_tests(home.path().to_path_buf()); + let (config, command_cwd) = + load_setup_config(&manager, install.path(), Some(project.path().to_path_buf())).await?; + let expected = AbsolutePathBuf::from_absolute_path(project.path())?; + + assert_eq!( + (command_cwd, config.effective_workspace_roots()), + (project.path().to_path_buf(), vec![expected]), + ); + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/elevated/runner_pipe.rs b/codex-rs/windows-sandbox-rs/src/elevated/runner_pipe.rs index c8ae092c4a..b23f7fd380 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/runner_pipe.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/runner_pipe.rs @@ -7,8 +7,7 @@ //! and elevated capture. The legacy restricted‑token path spawns the child directly //! and does not use these helpers. -use crate::helper_materialization::HelperExecutable; -use crate::helper_materialization::resolve_helper_for_launch; +use crate::helper_materialization::resolve_command_runner; use crate::winutil::resolve_sid; use crate::winutil::string_from_sid_bytes; use crate::winutil::to_wide; @@ -41,7 +40,7 @@ pub const PIPE_ACCESS_OUTBOUND: u32 = 0x0000_0002; /// Resolves the elevated command runner path, preferring the copied helper under /// `.sandbox-bin` and falling back to the legacy sibling lookup when needed. pub fn find_runner_exe(codex_home: &Path, log_dir: Option<&Path>) -> PathBuf { - resolve_helper_for_launch(HelperExecutable::CommandRunner, codex_home, log_dir) + resolve_command_runner(codex_home, log_dir) } /// Generates a unique named-pipe path used to communicate with the runner process. diff --git a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs index 7532d191ea..1ada0afaa2 100644 --- a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs +++ b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs @@ -1,75 +1,51 @@ +//! Selects sandbox helper paths; legacy file copying lives in the copy module. + +mod copy; +use copy::CopyOutcome; +use copy::copy_from_source_if_needed; + use anyhow::Context; use anyhow::Result; use anyhow::anyhow; use std::collections::HashMap; use std::ffi::OsStr; use std::fs; -use std::io::Write; + use std::path::Path; use std::path::PathBuf; use std::sync::Mutex; use std::sync::OnceLock; use std::time::UNIX_EPOCH; -use tempfile::NamedTempFile; use crate::logging::log_note; use crate::sandbox_bin_dir; const DEV_BUILD_VERSION_SENTINEL: &str = "0.0.0"; +const COMMAND_RUNNER_EXE: &str = "codex-command-runner.exe"; pub(crate) const BIN_DIRNAME: &str = "bin"; pub(crate) const RESOURCES_DIRNAME: &str = "codex-resources"; -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -pub(crate) enum HelperExecutable { - CommandRunner, -} - -impl HelperExecutable { - fn file_name(self) -> &'static str { - match self { - Self::CommandRunner => "codex-command-runner.exe", - } - } - - fn label(self) -> &'static str { - match self { - Self::CommandRunner => "command-runner", - } - } -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum CopyOutcome { - Reused, - ReCopied, -} - static HELPER_PATH_CACHE: OnceLock>> = OnceLock::new(); pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf { sandbox_bin_dir(codex_home) } -pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf { +pub(crate) fn legacy_lookup() -> PathBuf { if let Ok(exe) = std::env::current_exe() - && let Some(candidate) = bundled_executable_path_for_exe(&exe, kind.file_name()) + && let Some(candidate) = bundled_executable_path_for_exe(&exe, COMMAND_RUNNER_EXE) { return candidate; } - PathBuf::from(kind.file_name()) + PathBuf::from(COMMAND_RUNNER_EXE) } -pub(crate) fn resolve_helper_for_launch( - kind: HelperExecutable, - codex_home: &Path, - log_dir: Option<&Path>, -) -> PathBuf { - match copy_helper_if_needed(kind, codex_home, log_dir) { +pub(crate) fn resolve_command_runner(codex_home: &Path, log_dir: Option<&Path>) -> PathBuf { + match copy_runner_if_needed(codex_home, log_dir) { Ok(path) => { log_note( &format!( - "helper launch resolution: using copied {} path {}", - kind.label(), + "helper launch resolution: using copied command-runner path {}", path.display() ), log_dir, @@ -77,11 +53,10 @@ pub(crate) fn resolve_helper_for_launch( path } Err(err) => { - let fallback = legacy_lookup(kind); + let fallback = legacy_lookup(); log_note( &format!( - "helper copy failed for {}: {err:#}; falling back to legacy path {}", - kind.label(), + "helper copy failed for command-runner: {err:#}; falling back to legacy path {}", fallback.display() ), log_dir, @@ -120,17 +95,12 @@ pub fn resolve_exe_for_launch(source: &Path, codex_home: &Path) -> PathBuf { } } -pub(crate) fn copy_helper_if_needed( - kind: HelperExecutable, - codex_home: &Path, - log_dir: Option<&Path>, -) -> Result { - let cache_key = format!("{}|{}", kind.file_name(), codex_home.display()); +fn copy_runner_if_needed(codex_home: &Path, log_dir: Option<&Path>) -> Result { + let cache_key = format!("{}|{}", COMMAND_RUNNER_EXE, codex_home.display()); if let Some(path) = cached_helper_path(&cache_key) { log_note( &format!( - "helper copy: using in-memory cache for {} -> {}", - kind.label(), + "helper copy: using in-memory cache for command-runner -> {}", path.display() ), log_dir, @@ -138,12 +108,11 @@ pub(crate) fn copy_helper_if_needed( return Ok(path); } - let source = sibling_source_path(kind)?; - let destination = helper_destination_for_source(kind, codex_home, &source)?; + let source = sibling_source_path()?; + let destination = helper_destination_for_source(codex_home, &source)?; log_note( &format!( - "helper copy: validating {} source={} destination={}", - kind.label(), + "helper copy: validating command-runner source={} destination={}", source.display(), destination.display() ), @@ -156,9 +125,8 @@ pub(crate) fn copy_helper_if_needed( }; log_note( &format!( - "helper copy: {} {} source={} destination={}", + "helper copy: {} command-runner source={} destination={}", action, - kind.label(), source.display(), destination.display() ), @@ -181,9 +149,9 @@ fn store_helper_path(cache_key: String, path: PathBuf) { } } -fn sibling_source_path(kind: HelperExecutable) -> Result { +fn sibling_source_path() -> Result { let exe = std::env::current_exe().context("resolve current executable for helper lookup")?; - bundled_executable_path_for_exe(&exe, kind.file_name()).ok_or_else(|| { + bundled_executable_path_for_exe(&exe, COMMAND_RUNNER_EXE).ok_or_else(|| { anyhow!( "helper not found next to current executable or under {RESOURCES_DIRNAME}: {}", exe.display() @@ -216,29 +184,13 @@ pub(crate) fn bundled_executable_path_for_exe(exe: &Path, file_name: &str) -> Op find(exe).or_else(|| find(&dunce::canonicalize(exe).ok()?)) } -fn helper_destination_for_source( - kind: HelperExecutable, - codex_home: &Path, - source: &Path, -) -> Result { +fn helper_destination_for_source(codex_home: &Path, source: &Path) -> Result { let suffix = helper_version_suffix(source)?; - let file_name = materialized_file_name(kind, &suffix); - Ok(helper_bin_dir(codex_home).join(file_name)) + Ok(helper_bin_dir(codex_home).join(materialized_file_name(&suffix))) } -fn materialized_file_name(kind: HelperExecutable, suffix: &str) -> String { - let source_name = kind.file_name(); - let path = Path::new(source_name); - let stem = path - .file_stem() - .and_then(|stem| stem.to_str()) - .unwrap_or(source_name); - let extension = path - .extension() - .and_then(|ext| ext.to_str()) - .map(|ext| format!(".{ext}")) - .unwrap_or_default(); - format!("{stem}-{suffix}{extension}") +fn materialized_file_name(suffix: &str) -> String { + format!("codex-command-runner-{suffix}.exe") } fn helper_version_suffix(source: &Path) -> Result { @@ -262,117 +214,16 @@ fn dev_build_suffix(source: &Path) -> Result { Ok(format!("{}-{:x}", metadata.len(), duration.as_secs(),)) } -fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result { - if destination_is_fresh(source, destination)? { - return Ok(CopyOutcome::Reused); - } - - let destination_dir = destination.parent().ok_or_else(|| { - anyhow!( - "helper destination has no parent: {}", - destination.display() - ) - })?; - fs::create_dir_all(destination_dir).with_context(|| { - format!( - "create helper destination directory {}", - destination_dir.display() - ) - })?; - - let temp_path = NamedTempFile::new_in(destination_dir) - .with_context(|| { - format!( - "create temporary helper file in {}", - destination_dir.display() - ) - })? - .into_temp_path(); - let temp_path_buf = temp_path.to_path_buf(); - - let mut source_file = fs::File::open(source) - .with_context(|| format!("open helper source for read {}", source.display()))?; - let mut temp_file = fs::OpenOptions::new() - .write(true) - .truncate(true) - .open(&temp_path_buf) - .with_context(|| format!("open temporary helper file {}", temp_path_buf.display()))?; - - // Write into a temp file created inside `.sandbox-bin` so the copied helper keeps the - // destination directory's inherited ACLs instead of reusing the source file's descriptor. - std::io::copy(&mut source_file, &mut temp_file).with_context(|| { - format!( - "copy helper from {} to {}", - source.display(), - temp_path_buf.display() - ) - })?; - temp_file - .flush() - .with_context(|| format!("flush temporary helper file {}", temp_path_buf.display()))?; - drop(temp_file); - - if destination.exists() { - fs::remove_file(destination).with_context(|| { - format!("remove stale helper destination {}", destination.display()) - })?; - } - - match fs::rename(&temp_path_buf, destination) { - Ok(()) => Ok(CopyOutcome::ReCopied), - Err(rename_err) => { - if destination_is_fresh(source, destination)? { - Ok(CopyOutcome::Reused) - } else { - Err(rename_err).with_context(|| { - format!( - "rename helper temp file {} to {}", - temp_path_buf.display(), - destination.display() - ) - }) - } - } - } -} - -fn destination_is_fresh(source: &Path, destination: &Path) -> Result { - let source_meta = fs::metadata(source) - .with_context(|| format!("read helper source metadata {}", source.display()))?; - let destination_meta = match fs::metadata(destination) { - Ok(meta) => meta, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), - Err(err) => { - return Err(err).with_context(|| { - format!("read helper destination metadata {}", destination.display()) - }); - } - }; - - if source_meta.len() != destination_meta.len() { - return Ok(false); - } - - let source_modified = source_meta - .modified() - .with_context(|| format!("read helper source mtime {}", source.display()))?; - let destination_modified = destination_meta - .modified() - .with_context(|| format!("read helper destination mtime {}", destination.display()))?; - - Ok(destination_modified >= source_modified) -} - #[cfg(test)] mod tests { use super::BIN_DIRNAME; use super::CopyOutcome; use super::DEV_BUILD_VERSION_SENTINEL; - use super::HelperExecutable; + use super::RESOURCES_DIRNAME; use super::bundled_executable_path_for_exe; use super::copy_from_source_if_needed; - use super::destination_is_fresh; + use super::dev_build_suffix; use super::helper_bin_dir; use super::helper_version_suffix; @@ -383,56 +234,6 @@ mod tests { use std::path::PathBuf; use tempfile::TempDir; - #[test] - fn copy_from_source_if_needed_copies_missing_destination() { - let tmp = TempDir::new().expect("tempdir"); - let source = tmp.path().join("source.exe"); - let destination = tmp.path().join("bin").join("helper.exe"); - - fs::write(&source, b"runner-v1").expect("write source"); - - let outcome = copy_from_source_if_needed(&source, &destination).expect("copy helper"); - - assert_eq!(CopyOutcome::ReCopied, outcome); - assert_eq!( - b"runner-v1".as_slice(), - fs::read(&destination).expect("read destination") - ); - } - - #[test] - fn destination_is_fresh_uses_size_and_mtime() { - let tmp = TempDir::new().expect("tempdir"); - let source = tmp.path().join("source.exe"); - let destination = tmp.path().join("destination.exe"); - - fs::write(&destination, b"same-size").expect("write destination"); - std::thread::sleep(std::time::Duration::from_secs(1)); - fs::write(&source, b"same-size").expect("write source"); - assert!(!destination_is_fresh(&source, &destination).expect("stale metadata")); - - fs::write(&destination, b"same-size").expect("rewrite destination"); - assert!(destination_is_fresh(&source, &destination).expect("fresh metadata")); - } - - #[test] - fn copy_from_source_if_needed_reuses_fresh_destination() { - let tmp = TempDir::new().expect("tempdir"); - let source = tmp.path().join("source.exe"); - let destination = tmp.path().join("bin").join("helper.exe"); - - fs::write(&source, b"runner-v1").expect("write source"); - copy_from_source_if_needed(&source, &destination).expect("initial copy"); - - let outcome = copy_from_source_if_needed(&source, &destination).expect("revalidate helper"); - - assert_eq!(CopyOutcome::Reused, outcome); - assert_eq!( - b"runner-v1".as_slice(), - fs::read(&destination).expect("read destination") - ); - } - #[test] fn helper_bin_dir_is_under_sandbox_bin() { let codex_home = Path::new(r"C:\Users\example\.codex"); @@ -452,10 +253,8 @@ mod tests { let runner_source = source_dir.join("codex-command-runner.exe"); fs::write(&runner_source, b"runner").expect("runner"); let runner_suffix = helper_version_suffix(&runner_source).expect("runner suffix"); - let runner_destination = helper_bin_dir(&codex_home).join(materialized_file_name( - HelperExecutable::CommandRunner, - &runner_suffix, - )); + let runner_destination = + helper_bin_dir(&codex_home).join(materialized_file_name(&runner_suffix)); let runner_outcome = copy_from_source_if_needed(&runner_source, &runner_destination).expect("runner copy"); @@ -596,7 +395,7 @@ mod tests { #[test] fn materialized_file_name_adds_suffix_before_extension() { - let file_name = materialized_file_name(HelperExecutable::CommandRunner, "test-suffix"); + let file_name = materialized_file_name("test-suffix"); assert_eq!(file_name, "codex-command-runner-test-suffix.exe"); } diff --git a/codex-rs/windows-sandbox-rs/src/helper_materialization/copy.rs b/codex-rs/windows-sandbox-rs/src/helper_materialization/copy.rs new file mode 100644 index 0000000000..2658bd350c --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/helper_materialization/copy.rs @@ -0,0 +1,120 @@ +//! Copies legacy helpers using destination-inherited ACLs and the existing freshness check. + +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use std::fs; +use std::io::Write; +use std::path::Path; +use tempfile::NamedTempFile; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum CopyOutcome { + Reused, + ReCopied, +} + +pub(super) fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result { + if destination_is_fresh(source, destination)? { + return Ok(CopyOutcome::Reused); + } + + let destination_dir = destination.parent().ok_or_else(|| { + anyhow!( + "helper destination has no parent: {}", + destination.display() + ) + })?; + fs::create_dir_all(destination_dir).with_context(|| { + format!( + "create helper destination directory {}", + destination_dir.display() + ) + })?; + + let temp_path = NamedTempFile::new_in(destination_dir) + .with_context(|| { + format!( + "create temporary helper file in {}", + destination_dir.display() + ) + })? + .into_temp_path(); + let temp_path_buf = temp_path.to_path_buf(); + + let mut source_file = fs::File::open(source) + .with_context(|| format!("open helper source for read {}", source.display()))?; + let mut temp_file = fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(&temp_path_buf) + .with_context(|| format!("open temporary helper file {}", temp_path_buf.display()))?; + + // Write into a temp file created inside `.sandbox-bin` so the copied helper keeps the + // destination directory's inherited ACLs instead of reusing the source file's descriptor. + std::io::copy(&mut source_file, &mut temp_file).with_context(|| { + format!( + "copy helper from {} to {}", + source.display(), + temp_path_buf.display() + ) + })?; + temp_file + .flush() + .with_context(|| format!("flush temporary helper file {}", temp_path_buf.display()))?; + drop(temp_file); + + if destination.exists() { + fs::remove_file(destination).with_context(|| { + format!("remove stale helper destination {}", destination.display()) + })?; + } + + match fs::rename(&temp_path_buf, destination) { + Ok(()) => Ok(CopyOutcome::ReCopied), + Err(rename_err) => { + if destination_is_fresh(source, destination)? { + Ok(CopyOutcome::Reused) + } else { + Err(rename_err).with_context(|| { + format!( + "rename helper temp file {} to {}", + temp_path_buf.display(), + destination.display() + ) + }) + } + } + } +} + +fn destination_is_fresh(source: &Path, destination: &Path) -> Result { + let source_meta = fs::metadata(source) + .with_context(|| format!("read helper source metadata {}", source.display()))?; + let destination_meta = match fs::metadata(destination) { + Ok(meta) => meta, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(err) => { + return Err(err).with_context(|| { + format!("read helper destination metadata {}", destination.display()) + }); + } + }; + + if source_meta.len() != destination_meta.len() { + return Ok(false); + } + + let source_modified = source_meta + .modified() + .with_context(|| format!("read helper source mtime {}", source.display()))?; + let destination_modified = destination_meta + .modified() + .with_context(|| format!("read helper destination mtime {}", destination.display()))?; + + Ok(destination_modified >= source_modified) +} + +#[cfg(test)] +#[path = "copy_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/helper_materialization/copy_tests.rs b/codex-rs/windows-sandbox-rs/src/helper_materialization/copy_tests.rs new file mode 100644 index 0000000000..8a71d392a2 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/helper_materialization/copy_tests.rs @@ -0,0 +1,58 @@ +//! Tests the legacy helper copy and freshness contract. + +use super::CopyOutcome; +use super::copy_from_source_if_needed; +use super::destination_is_fresh; +use pretty_assertions::assert_eq; +use std::fs; +use tempfile::TempDir; + +#[test] +fn copy_from_source_if_needed_copies_missing_destination() { + let tmp = TempDir::new().expect("tempdir"); + let source = tmp.path().join("source.exe"); + let destination = tmp.path().join("bin").join("helper.exe"); + + fs::write(&source, b"runner-v1").expect("write source"); + + let outcome = copy_from_source_if_needed(&source, &destination).expect("copy helper"); + + assert_eq!(CopyOutcome::ReCopied, outcome); + assert_eq!( + b"runner-v1".as_slice(), + fs::read(&destination).expect("read destination") + ); +} + +#[test] +fn destination_is_fresh_uses_size_and_mtime() { + let tmp = TempDir::new().expect("tempdir"); + let source = tmp.path().join("source.exe"); + let destination = tmp.path().join("destination.exe"); + + fs::write(&destination, b"same-size").expect("write destination"); + std::thread::sleep(std::time::Duration::from_secs(1)); + fs::write(&source, b"same-size").expect("write source"); + assert!(!destination_is_fresh(&source, &destination).expect("stale metadata")); + + fs::write(&destination, b"same-size").expect("rewrite destination"); + assert!(destination_is_fresh(&source, &destination).expect("fresh metadata")); +} + +#[test] +fn copy_from_source_if_needed_reuses_fresh_destination() { + let tmp = TempDir::new().expect("tempdir"); + let source = tmp.path().join("source.exe"); + let destination = tmp.path().join("bin").join("helper.exe"); + + fs::write(&source, b"runner-v1").expect("write source"); + copy_from_source_if_needed(&source, &destination).expect("initial copy"); + + let outcome = copy_from_source_if_needed(&source, &destination).expect("revalidate helper"); + + assert_eq!(CopyOutcome::Reused, outcome); + assert_eq!( + b"runner-v1".as_slice(), + fs::read(&destination).expect("read destination") + ); +} diff --git a/codex-rs/windows-sandbox-rs/src/installation_record.rs b/codex-rs/windows-sandbox-rs/src/installation_record.rs index 3a065cd615..84a1fee295 100644 --- a/codex-rs/windows-sandbox-rs/src/installation_record.rs +++ b/codex-rs/windows-sandbox-rs/src/installation_record.rs @@ -37,12 +37,16 @@ pub struct InstallationRecord { } pub fn load() -> Result> { + load_from(INSTALLATION_KEY) +} + +pub(crate) fn load_from(key: &str) -> Result> { let mut value = [0_u16; MAX_VALUE_UNITS]; let mut value_length = std::mem::size_of_val(&value) as u32; let status = unsafe { registry::RegGetValueW( registry::HKEY_LOCAL_MACHINE, - to_wide(INSTALLATION_KEY).as_ptr(), + to_wide(key).as_ptr(), to_wide(INSTALLATION_VALUE).as_ptr(), registry::RRF_RT_REG_SZ, ptr::null_mut(), @@ -72,6 +76,10 @@ pub fn load() -> Result> { } pub fn save(record: &InstallationRecord) -> Result<()> { + save_to(INSTALLATION_KEY, record) +} + +pub(crate) fn save_to(key: &str, record: &InstallationRecord) -> Result<()> { let value = to_wide( serde_json::to_string(record).context("serialize protected sandbox installation record")?, ); @@ -82,7 +90,7 @@ pub fn save(record: &InstallationRecord) -> Result<()> { let status = unsafe { registry::RegSetKeyValueW( registry::HKEY_LOCAL_MACHINE, - to_wide(INSTALLATION_KEY).as_ptr(), + to_wide(key).as_ptr(), to_wide(INSTALLATION_VALUE).as_ptr(), registry::REG_SZ, value.as_ptr().cast(), diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 1478caef32..26323dec30 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -97,6 +97,8 @@ mod resolved_permissions; #[cfg(target_os = "windows")] mod token; #[cfg(target_os = "windows")] +mod token_user; +#[cfg(target_os = "windows")] mod wfp; #[cfg(target_os = "windows")] mod wfp_setup; diff --git a/codex-rs/windows-sandbox-rs/src/provisioning_client.rs b/codex-rs/windows-sandbox-rs/src/provisioning_client.rs index b80e094c1c..c56e844ee4 100644 --- a/codex-rs/windows-sandbox-rs/src/provisioning_client.rs +++ b/codex-rs/windows-sandbox-rs/src/provisioning_client.rs @@ -119,7 +119,7 @@ pub fn provision_windows_sandbox_via_service( }, }; - match send_service_request(request, PROVISIONING_TIMEOUT)? { + match send_service_request(&request, PROVISIONING_TIMEOUT)? { crate::SandboxProvisioningResponse::Ok => { Ok(WindowsSandboxProvisioningOutcome::Provisioned) } @@ -141,7 +141,7 @@ pub fn register_desktop_installation(codex_home: &Path) -> anyhow::Result<()> { .to_owned(), }, }; - match send_service_request(request, Duration::from_secs(5))? { + match send_service_request(&request, Duration::from_secs(5))? { crate::SandboxProvisioningResponse::Ok => Ok(()), crate::SandboxProvisioningResponse::Unavailable => { bail!("desktop uninstall registration service is unavailable") @@ -151,31 +151,14 @@ pub fn register_desktop_installation(codex_home: &Path) -> anyhow::Result<()> { } fn send_service_request( - request: crate::FramedProvisioningMessage, + request: &crate::FramedProvisioningMessage, timeout: Duration, ) -> anyhow::Result { let deadline = Instant::now() + timeout; let Some(mut pipe) = connect(deadline)? else { return Ok(crate::SandboxProvisioningResponse::Unavailable); }; - let response = (|| -> anyhow::Result { - verify_server(pipe.as_raw_handle() as HANDLE) - .context("authenticate provisioning pipe server")?; - crate::write_provisioning_frame(&mut pipe, &request) - .context("send sandbox provisioning request")?; - crate::framed_io::wait_for_complete_frame(&pipe, deadline) - .context("wait for sandbox provisioning response")?; - crate::read_provisioning_frame(&mut pipe) - .context("read sandbox provisioning response")? - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::UnexpectedEof, - "sandbox provisioning service closed the pipe without a response", - ) - }) - .context("read sandbox provisioning response") - })(); - let response = match response { + let response = match exchange_request(&mut pipe, request, deadline) { Ok(response) => response, Err(error) if error.downcast_ref::().is_some_and(|error| { @@ -195,7 +178,29 @@ fn send_service_request( } Err(error) => return Err(error), }; + Ok(response) +} +fn exchange_request( + pipe: &mut File, + request: &crate::FramedProvisioningMessage, + deadline: Instant, +) -> anyhow::Result { + verify_server(pipe.as_raw_handle() as HANDLE) + .context("authenticate provisioning pipe server")?; + crate::write_provisioning_frame(&mut *pipe, request) + .context("send sandbox provisioning request")?; + crate::framed_io::wait_for_complete_frame(pipe, deadline) + .context("wait for sandbox provisioning response")?; + let response = crate::read_provisioning_frame(pipe) + .context("read sandbox provisioning response")? + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::UnexpectedEof, + "sandbox provisioning service closed the pipe without a response", + ) + }) + .context("read sandbox provisioning response")?; if response.version != crate::PROVISIONING_PROTOCOL_VERSION { return Ok(crate::SandboxProvisioningResponse::Unavailable); } diff --git a/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs b/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs index 13faad8ef5..2f0c7862cb 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs @@ -519,6 +519,10 @@ fn real_main(setup_mode: &mut Option) -> Result<()> { ), ))); } + run_payload(&payload) +} + +fn run_payload(payload: &Payload) -> Result<()> { let sbx_dir = sandbox_dir(&payload.codex_home); std::fs::create_dir_all(&sbx_dir).map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -532,7 +536,7 @@ fn real_main(setup_mode: &mut Option) -> Result<()> { format!("open log in {} failed: {err}", sbx_dir.display()), )) })?; - let result = run_setup(&payload, &mut log, &sbx_dir); + let result = run_setup(payload, &mut log, &sbx_dir); if let Err(err) = &result { let _ = log_line(&mut log, &format!("setup error: {err:?}")); log_note(&format!("setup error: {err:?}"), Some(sbx_dir.as_path())); diff --git a/codex-rs/windows-sandbox-rs/src/token.rs b/codex-rs/windows-sandbox-rs/src/token.rs index c7897f3d91..32d69af82a 100644 --- a/codex-rs/windows-sandbox-rs/src/token.rs +++ b/codex-rs/windows-sandbox-rs/src/token.rs @@ -20,7 +20,6 @@ use windows_sys::Win32::Security::Authorization::TRUSTEE_W; use windows_sys::Win32::Security::CopySid; use windows_sys::Win32::Security::CreateRestrictedToken; use windows_sys::Win32::Security::CreateWellKnownSid; -use windows_sys::Win32::Security::GetLengthSid; use windows_sys::Win32::Security::GetTokenInformation; use windows_sys::Win32::Security::IsValidSid; use windows_sys::Win32::Security::LookupPrivilegeValueW; @@ -36,10 +35,8 @@ use windows_sys::Win32::Security::TOKEN_DUPLICATE; use windows_sys::Win32::Security::TOKEN_GROUPS; use windows_sys::Win32::Security::TOKEN_PRIVILEGES; use windows_sys::Win32::Security::TOKEN_QUERY; -use windows_sys::Win32::Security::TOKEN_USER; use windows_sys::Win32::Security::TokenDefaultDacl; use windows_sys::Win32::Security::TokenGroups; -use windows_sys::Win32::Security::TokenUser; use windows_sys::Win32::System::Threading::GetCurrentProcess; const DISABLE_MAX_PRIVILEGE: u32 = 0x01; @@ -332,45 +329,7 @@ pub unsafe fn get_logon_sid_bytes(h_token: HANDLE) -> Result> { Err(anyhow!("Logon SID not present on token")) } -pub(crate) unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result> { - let mut needed: u32 = 0; - GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed); - if needed == 0 { - return Err(anyhow!("TokenUser size query returned 0")); - } - let mut user_buf: Vec = vec![0u8; needed as usize]; - let ok = GetTokenInformation( - h_token, - TokenUser, - user_buf.as_mut_ptr() as *mut c_void, - needed, - &mut needed, - ); - if ok == 0 || (needed as usize) < std::mem::size_of::() { - return Err(anyhow!( - "GetTokenInformation(TokenUser) failed: {}", - GetLastError() - )); - } - let token_user: TOKEN_USER = std::ptr::read_unaligned(user_buf.as_ptr() as *const TOKEN_USER); - let sid_len = GetLengthSid(token_user.User.Sid); - if sid_len == 0 { - return Err(anyhow!( - "GetLengthSid(TokenUser) failed: {}", - GetLastError() - )); - } - let mut user_sid_bytes = vec![0u8; sid_len as usize]; - if CopySid( - sid_len, - user_sid_bytes.as_mut_ptr() as *mut c_void, - token_user.User.Sid, - ) == 0 - { - return Err(anyhow!("CopySid(TokenUser) failed: {}", GetLastError())); - } - Ok(user_sid_bytes) -} +pub(crate) use crate::token_user::get_user_sid_bytes; unsafe fn enable_single_privilege(h_token: HANDLE, name: &str) -> Result<()> { let mut luid = LUID { diff --git a/codex-rs/windows-sandbox-rs/src/token_user.rs b/codex-rs/windows-sandbox-rs/src/token_user.rs new file mode 100644 index 0000000000..2b511b5d40 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/token_user.rs @@ -0,0 +1,56 @@ +//! Owns the token-user SID query used by sandbox token construction. + +use anyhow::Result; +use anyhow::anyhow; +use std::ffi::c_void; +use windows_sys::Win32::Foundation::GetLastError; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Security::CopySid; +use windows_sys::Win32::Security::GetLengthSid; +use windows_sys::Win32::Security::GetTokenInformation; +use windows_sys::Win32::Security::TOKEN_USER; +use windows_sys::Win32::Security::TokenUser; + +pub(crate) unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result> { + let mut needed: u32 = 0; + GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed); + if needed == 0 { + return Err(anyhow!("TokenUser size query returned 0")); + } + let mut user_buf: Vec = vec![0u8; needed as usize]; + let ok = GetTokenInformation( + h_token, + TokenUser, + user_buf.as_mut_ptr() as *mut c_void, + needed, + &mut needed, + ); + if ok == 0 || (needed as usize) < std::mem::size_of::() { + return Err(anyhow!( + "GetTokenInformation(TokenUser) failed: {}", + GetLastError() + )); + } + let token_user: TOKEN_USER = std::ptr::read_unaligned(user_buf.as_ptr() as *const TOKEN_USER); + let sid_len = GetLengthSid(token_user.User.Sid); + if sid_len == 0 { + return Err(anyhow!( + "GetLengthSid(TokenUser) failed: {}", + GetLastError() + )); + } + let mut user_sid_bytes = vec![0u8; sid_len as usize]; + if CopySid( + sid_len, + user_sid_bytes.as_mut_ptr() as *mut c_void, + token_user.User.Sid, + ) == 0 + { + return Err(anyhow!("CopySid(TokenUser) failed: {}", GetLastError())); + } + Ok(user_sid_bytes) +} + +#[cfg(test)] +#[path = "token_user_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/token_user_tests.rs b/codex-rs/windows-sandbox-rs/src/token_user_tests.rs new file mode 100644 index 0000000000..b062d52e6e --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/token_user_tests.rs @@ -0,0 +1,22 @@ +//! Checks the existing token-user query against real process and invalid handles. + +use super::get_user_sid_bytes; +use anyhow::Result; +use anyhow::ensure; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use windows_sys::Win32::Security::IsValidSid; +use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::System::Threading::GetCurrentProcess; +use windows_sys::Win32::System::Threading::OpenProcessToken; + +#[test] +fn queries_current_user_and_rejects_invalid_token() -> Result<()> { + let mut raw = 0; + ensure!(unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut raw) } != 0); + let _token = unsafe { OwnedHandle::from_raw_handle(raw as _) }; + let user = unsafe { get_user_sid_bytes(raw) }?; + assert!(unsafe { IsValidSid(user.as_ptr() as _) } != 0); + assert!(unsafe { get_user_sid_bytes(0) }.is_err()); + Ok(()) +} diff --git a/codex-rs/windows-sandbox-service/src/ipc.rs b/codex-rs/windows-sandbox-service/src/ipc.rs index a1ff38eb84..201cdce4f3 100644 --- a/codex-rs/windows-sandbox-service/src/ipc.rs +++ b/codex-rs/windows-sandbox-service/src/ipc.rs @@ -5,6 +5,7 @@ mod authentication; mod home; +mod listener; mod request; use anyhow::Context; @@ -16,8 +17,6 @@ use codex_windows_sandbox::FramedProvisioningMessage; use codex_windows_sandbox::PROVISIONING_PROTOCOL_VERSION; use codex_windows_sandbox::ProvisioningMessage; use codex_windows_sandbox::SandboxProvisioningResponse; -use codex_windows_sandbox::ensure_sandbox_users_group; -use codex_windows_sandbox::string_from_sid_bytes; use codex_windows_sandbox::to_wide; use codex_windows_sandbox::write_provisioning_frame; pub(crate) use home::OwnedHandle; @@ -36,8 +35,6 @@ use std::time::Duration; use std::time::Instant; use windows_sys::Win32::Foundation as foundation; use windows_sys::Win32::Foundation::HANDLE; -use windows_sys::Win32::Security as security; -use windows_sys::Win32::Security::Authorization as authorization; use windows_sys::Win32::Storage::FileSystem as filesystem; use windows_sys::Win32::System::Pipes as pipes; @@ -62,14 +59,6 @@ impl std::fmt::Display for ServiceUnavailable { impl std::error::Error for ServiceUnavailable {} -struct SecurityDescriptor(security::PSECURITY_DESCRIPTOR); - -impl Drop for SecurityDescriptor { - fn drop(&mut self) { - unsafe { foundation::LocalFree(self.0 as foundation::HLOCAL) }; - } -} - #[derive(Debug, Eq, PartialEq)] enum PipeConnection { Connected, @@ -79,54 +68,14 @@ enum PipeConnection { pub(crate) fn run( shutdown: Arc, on_ready: impl FnOnce() -> Result<()>, - register_installation: impl Fn(InstallationRecord, OwnedHandle) -> Result<()>, + register_installation: impl Fn(InstallationRecord, OwnedHandle) -> Result, on_session_change: impl Fn() -> Result<()>, ) -> Result<()> { - let sandbox_sid = ensure_sandbox_users_group()?; - let sid_string = string_from_sid_bytes(&sandbox_sid).map_err(anyhow::Error::msg)?; - let sddl = pipe_security_descriptor(&sid_string); - let mut descriptor: security::PSECURITY_DESCRIPTOR = ptr::null_mut(); - if unsafe { - authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW( - to_wide(sddl).as_ptr(), - authorization::SDDL_REVISION_1, - &mut descriptor, - ptr::null_mut(), - ) - } == 0 - { - return Err(std::io::Error::last_os_error()).context("create provisioning pipe DACL"); - } - let descriptor = SecurityDescriptor(descriptor); - let attributes = security::SECURITY_ATTRIBUTES { - nLength: size_of::() as u32, - lpSecurityDescriptor: descriptor.0, - bInheritHandle: 0, - }; - - let pipe = unsafe { - pipes::CreateNamedPipeW( - to_wide(PIPE_NAME).as_ptr(), - filesystem::PIPE_ACCESS_DUPLEX | filesystem::FILE_FLAG_FIRST_PIPE_INSTANCE, - pipes::PIPE_TYPE_BYTE - | pipes::PIPE_READMODE_BYTE - | pipes::PIPE_WAIT - | pipes::PIPE_REJECT_REMOTE_CLIENTS, - 1, - 1024, - MAX_REQUEST_BYTES as u32, - 0, - &attributes, - ) - }; - if pipe == foundation::INVALID_HANDLE_VALUE { - return Err(std::io::Error::last_os_error()).context("create provisioning pipe"); - } - let pipe = OwnedHandle(pipe); + let listener = listener::ProvisioningListener::open()?; on_ready().context("publish provisioning listener readiness")?; while !shutdown.load(Ordering::Acquire) { - let connection = accept_pipe_connection(pipe.0)?; + let connection = accept_pipe_connection(listener.pipe.0)?; if shutdown.load(Ordering::Acquire) { break; } @@ -136,17 +85,18 @@ pub(crate) fn run( continue; } - let authorized_process = match crate::package_identity::authorize_client_process(pipe.0) { - Ok(process) => process, - Err(_) => { - unsafe { pipes::DisconnectNamedPipe(pipe.0) }; - continue; - } - }; + let authorized_process = + match crate::package_identity::authorize_client_process(listener.pipe.0) { + Ok(process) => process, + Err(_) => { + unsafe { pipes::DisconnectNamedPipe(listener.pipe.0) }; + continue; + } + }; let result = handle_request( - pipe.0, + listener.pipe.0, &authorized_process, - &sandbox_sid, + &listener.sandbox_sid, &shutdown, ®ister_installation, ); @@ -158,62 +108,78 @@ pub(crate) fn run( } Err(error) => { eprintln!("sandbox provisioning request failed: {error}"); - let mut message = String::new(); - for character in error.to_string().chars() { - let character = if character.is_control() { - ' ' - } else { - character - }; - if message.len() + character.len_utf8() > MAX_RESPONSE_MESSAGE_BYTES { - break; - } - message.push(character); + SandboxProvisioningResponse::Error { + message: response_error_message(&error), } - SandboxProvisioningResponse::Error { message } } }; let response = FramedProvisioningMessage { version: PROVISIONING_PROTOCOL_VERSION, message: ProvisioningMessage::ProvisionSandboxResponse { payload: response }, }; - let mut frame = Vec::new(); - write_provisioning_frame(&mut frame, &response) - .context("serialize sandbox provisioning response")?; - let mut written = 0; - let sent = unsafe { - filesystem::WriteFile( - pipe.0, - frame.as_ptr(), - frame.len() as u32, - &mut written, - ptr::null_mut(), - ) - }; - if sent != 0 { - let deadline = Instant::now() + Duration::from_secs(1); - while !shutdown.load(Ordering::Acquire) && Instant::now() < deadline { - if unsafe { - pipes::PeekNamedPipe( - pipe.0, - ptr::null_mut(), - 0, - ptr::null_mut(), - ptr::null_mut(), - ptr::null_mut(), - ) - } == 0 - { - break; - } - std::thread::sleep(Duration::from_millis(10)); - } - } - unsafe { pipes::DisconnectNamedPipe(pipe.0) }; + write_response(&listener.pipe, &response, &shutdown)?; + unsafe { pipes::DisconnectNamedPipe(listener.pipe.0) }; } Ok(()) } +/// Sends one frame, then waits briefly for the client to close before disconnecting. +fn write_response( + pipe: &OwnedHandle, + response: &FramedProvisioningMessage, + shutdown: &AtomicBool, +) -> Result<()> { + let mut frame = Vec::new(); + write_provisioning_frame(&mut frame, response) + .context("serialize sandbox provisioning response")?; + let mut written = 0; + let sent = unsafe { + filesystem::WriteFile( + pipe.0, + frame.as_ptr(), + frame.len() as u32, + &mut written, + ptr::null_mut(), + ) + }; + if sent != 0 { + let deadline = Instant::now() + Duration::from_secs(1); + while !shutdown.load(Ordering::Acquire) && Instant::now() < deadline { + if unsafe { + pipes::PeekNamedPipe( + pipe.0, + ptr::null_mut(), + 0, + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ) + } == 0 + { + break; + } + std::thread::sleep(Duration::from_millis(10)); + } + } + Ok(()) +} + +fn response_error_message(error: &anyhow::Error) -> String { + let mut message = String::new(); + for character in error.to_string().chars() { + let character = if character.is_control() { + ' ' + } else { + character + }; + if message.len() + character.len_utf8() > MAX_RESPONSE_MESSAGE_BYTES { + break; + } + message.push(character); + } + message +} + fn accept_pipe_connection(pipe: HANDLE) -> Result { if unsafe { pipes::ConnectNamedPipe(pipe, ptr::null_mut()) } != 0 { return Ok(PipeConnection::Connected); @@ -268,7 +234,7 @@ fn handle_request( authorized_process: &crate::package_identity::AuthorizedClientProcess, sandbox_sid: &[u8], shutdown: &AtomicBool, - register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result<()>, + register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result, ) -> Result { let deadline = Instant::now() + REQUEST_IDLE_TIMEOUT; let mut request = [0_u8; MAX_REQUEST_BYTES]; diff --git a/codex-rs/windows-sandbox-service/src/ipc/listener.rs b/codex-rs/windows-sandbox-service/src/ipc/listener.rs new file mode 100644 index 0000000000..d03e74bdd2 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/ipc/listener.rs @@ -0,0 +1,91 @@ +//! Owns the provisioning pipe and its security descriptor for the listener lifetime. + +use anyhow::Context; +use anyhow::Result; +use codex_windows_sandbox::ensure_sandbox_users_group; +use codex_windows_sandbox::string_from_sid_bytes; +use codex_windows_sandbox::to_wide; +use std::mem::size_of; +use std::ptr; +use windows_sys::Win32::Foundation as foundation; +use windows_sys::Win32::Security as security; +use windows_sys::Win32::Security::Authorization as authorization; +use windows_sys::Win32::Storage::FileSystem as filesystem; +use windows_sys::Win32::System::Pipes as pipes; + +use super::MAX_REQUEST_BYTES; +use super::OwnedHandle; +use super::pipe_security_descriptor; + +pub(super) struct ProvisioningListener { + pub(super) pipe: OwnedHandle, + pub(super) sandbox_sid: Vec, + _descriptor: SecurityDescriptor, +} + +impl ProvisioningListener { + pub(super) fn open() -> Result { + let sandbox_sid = ensure_sandbox_users_group()?; + let (descriptor, pipe) = create_provisioning_pipe(super::PIPE_NAME, &sandbox_sid)?; + Ok(Self { + pipe, + sandbox_sid, + _descriptor: descriptor, + }) + } +} + +struct SecurityDescriptor(security::PSECURITY_DESCRIPTOR); + +impl Drop for SecurityDescriptor { + fn drop(&mut self) { + unsafe { foundation::LocalFree(self.0 as foundation::HLOCAL) }; + } +} + +/// Preserves the existing listener and descriptor lifetimes. +fn create_provisioning_pipe( + pipe_name: &str, + sandbox_sid: &[u8], +) -> Result<(SecurityDescriptor, OwnedHandle)> { + let sid_string = string_from_sid_bytes(sandbox_sid).map_err(anyhow::Error::msg)?; + let sddl = pipe_security_descriptor(&sid_string); + let mut descriptor: security::PSECURITY_DESCRIPTOR = ptr::null_mut(); + if unsafe { + authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW( + to_wide(sddl).as_ptr(), + authorization::SDDL_REVISION_1, + &mut descriptor, + ptr::null_mut(), + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("create provisioning pipe DACL"); + } + let descriptor = SecurityDescriptor(descriptor); + let attributes = security::SECURITY_ATTRIBUTES { + nLength: size_of::() as u32, + lpSecurityDescriptor: descriptor.0, + bInheritHandle: 0, + }; + + let pipe = unsafe { + pipes::CreateNamedPipeW( + to_wide(pipe_name).as_ptr(), + filesystem::PIPE_ACCESS_DUPLEX | filesystem::FILE_FLAG_FIRST_PIPE_INSTANCE, + pipes::PIPE_TYPE_BYTE + | pipes::PIPE_READMODE_BYTE + | pipes::PIPE_WAIT + | pipes::PIPE_REJECT_REMOTE_CLIENTS, + 1, + 1024, + MAX_REQUEST_BYTES as u32, + 0, + &attributes, + ) + }; + if pipe == foundation::INVALID_HANDLE_VALUE { + return Err(std::io::Error::last_os_error()).context("create provisioning pipe"); + } + Ok((descriptor, OwnedHandle(pipe))) +} diff --git a/codex-rs/windows-sandbox-service/src/package_lifecycle.rs b/codex-rs/windows-sandbox-service/src/package_lifecycle.rs index 7beae074f6..1117837e7b 100644 --- a/codex-rs/windows-sandbox-service/src/package_lifecycle.rs +++ b/codex-rs/windows-sandbox-service/src/package_lifecycle.rs @@ -68,7 +68,7 @@ impl PackageLifecycle { &self, mut record: InstallationRecord, user_token: OwnedHandle, - ) -> Result<()> { + ) -> Result { let mut active = self.installation.borrow_mut(); let previous = match active.as_ref() { Some(installation) => Some(installation.record.clone()), @@ -91,10 +91,11 @@ impl PackageLifecycle { && installation.codex_home.is_some() { // A restored watcher must immediately use newly registered desktop ownership. - installation.record = record; - return Ok(()); + installation.record = record.clone(); + return Ok(record); } + let saved_record = record.clone(); with_owner_impersonation(user_token.0, || { let mut directory_handles = Vec::new(); let codex_home = match crate::ipc::pin_existing_ancestors( @@ -163,7 +164,8 @@ impl PackageLifecycle { token, }); Ok(()) - }) + })?; + Ok(saved_record) } pub(crate) fn restore_logged_in_owner(&self, recorded_session_id: u32) -> Result<()> { @@ -231,6 +233,7 @@ impl PackageLifecycle { }, token, ) + .map(|_| ()) } pub(crate) fn clean_up(&self) -> Result<()> { diff --git a/codex-rs/windows-sandbox-service/src/provisioning.rs b/codex-rs/windows-sandbox-service/src/provisioning.rs index 0ca2f9cea6..23f2e66c96 100644 --- a/codex-rs/windows-sandbox-service/src/provisioning.rs +++ b/codex-rs/windows-sandbox-service/src/provisioning.rs @@ -20,7 +20,7 @@ use crate::ipc::ServiceRequest; pub(crate) fn run( identity: ClientIdentity, request: ServiceRequest, - register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result<()>, + register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result, ) -> Result { // A policy-rejected request must not choose the uninstall owner. Use the // token already authenticated above instead of impersonating the pipe again. diff --git a/codex-rs/windows-sandbox-service/src/service.rs b/codex-rs/windows-sandbox-service/src/service.rs index 870c3b1e09..6c188fc877 100644 --- a/codex-rs/windows-sandbox-service/src/service.rs +++ b/codex-rs/windows-sandbox-service/src/service.rs @@ -40,6 +40,8 @@ use windows_sys::Win32::System::Services::SERVICE_WIN32_OWN_PROCESS; use windows_sys::Win32::System::Services::SetServiceStatus; use windows_sys::Win32::System::Services::StartServiceCtrlDispatcherW; +mod runtime_lifecycle; + pub(crate) const SERVICE_NAME: &str = "CodexSandboxService"; const EVENT_SERVICE_STARTED: u32 = 1000; const EVENT_SERVICE_STOP_REQUESTED: u32 = 1001; @@ -109,7 +111,7 @@ pub(crate) fn run_foreground() -> Result<()> { eprintln!("{SERVICE_NAME} listening on {}", crate::ipc::PIPE_NAME); Ok(()) }, - |_, _| Ok(()), + |installation, _| Ok(installation), || Ok(()), ) } @@ -156,46 +158,7 @@ fn service_main_inner(state: &ServiceState) -> Result<()> { state.report_status(SERVICE_START_PENDING, NO_ERROR)?; let package_lifecycle = crate::package_lifecycle::PackageLifecycle::new(Arc::clone(&state.uninstalling))?; - crate::ipc::run( - Arc::clone(&state.shutdown), - || { - state.report_status(SERVICE_RUNNING, NO_ERROR)?; - if let Some(record) = crate::installation_record::load()? - && let Err(error) = package_lifecycle.restore_logged_in_owner(record.session_id) - { - log_error( - EVENT_SERVICE_FAILED, - &format!("unable to restore package uninstall listener: {error:#}"), - ); - } - log_information( - EVENT_SERVICE_STARTED, - "The Codex sandbox service is running.", - ); - Ok(()) - }, - |installation, user_token| { - package_lifecycle.register_authenticated_user(installation, user_token) - }, - || { - let session = state.changed_session.swap(u32::MAX, Ordering::AcqRel); - if session == u32::MAX { - return Ok(()); - } - if let Err(error) = package_lifecycle.restore_authenticated_user(session) { - log_error( - EVENT_SERVICE_FAILED, - &format!("unable to restore package uninstall listener: {error:#}"), - ); - } - Ok(()) - }, - ) - .context("run the sandbox provisioning broker")?; - - if state.stop_requested.load(Ordering::Acquire) && state.uninstalling.load(Ordering::Acquire) { - package_lifecycle.clean_up()?; - } + runtime_lifecycle::run(state, &package_lifecycle)?; log_information( EVENT_SERVICE_STOPPED, "The Codex sandbox service has stopped.", @@ -226,11 +189,7 @@ unsafe extern "system" fn service_control_handler( EVENT_SERVICE_STOP_REQUESTED, "The Codex sandbox service was asked to stop.", ); - std::thread::spawn(move || { - crate::ipc::wake(crate::ipc::PIPE_NAME, || { - state.current_status.load(Ordering::Acquire) == SERVICE_STOPPED - }); - }); + wake_listener(); } NO_ERROR } @@ -252,11 +211,7 @@ unsafe extern "system" fn service_control_handler( state .changed_session .store(event.dwSessionId, Ordering::Release); - std::thread::spawn(move || { - crate::ipc::wake(crate::ipc::PIPE_NAME, || { - state.current_status.load(Ordering::Acquire) == SERVICE_STOPPED - }); - }); + wake_listener(); } } NO_ERROR @@ -265,6 +220,16 @@ unsafe extern "system" fn service_control_handler( } } +fn wake_listener() { + if let Some(state) = SERVICE_STATE.get() { + std::thread::spawn(move || { + crate::ipc::wake(crate::ipc::PIPE_NAME, || { + state.current_status.load(Ordering::Acquire) == SERVICE_STOPPED + }); + }); + } +} + pub(crate) fn log_information(event_id: u32, message: &str) { log_event(EVENTLOG_INFORMATION_TYPE, event_id, message); } diff --git a/codex-rs/windows-sandbox-service/src/service/runtime_lifecycle.rs b/codex-rs/windows-sandbox-service/src/service/runtime_lifecycle.rs new file mode 100644 index 0000000000..4a7eadcc84 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/service/runtime_lifecycle.rs @@ -0,0 +1,60 @@ +//! Connects the provisioning broker to the existing package owner and uninstall listener. + +use std::sync::Arc; +use std::sync::atomic::Ordering; + +use anyhow::Context; +use anyhow::Result; +use windows_sys::Win32::Foundation::NO_ERROR; +use windows_sys::Win32::System::Services::SERVICE_RUNNING; + +use super::EVENT_SERVICE_FAILED; +use super::EVENT_SERVICE_STARTED; +use super::ServiceState; +use super::log_error; +use super::log_information; +use crate::package_lifecycle::PackageLifecycle; + +pub(super) fn run(state: &ServiceState, package_lifecycle: &PackageLifecycle) -> Result<()> { + crate::ipc::run( + Arc::clone(&state.shutdown), + || { + state.report_status(SERVICE_RUNNING, NO_ERROR)?; + if let Some(record) = crate::installation_record::load()? + && let Err(error) = package_lifecycle.restore_logged_in_owner(record.session_id) + { + log_error( + EVENT_SERVICE_FAILED, + &format!("unable to restore package uninstall listener: {error:#}"), + ); + } + log_information( + EVENT_SERVICE_STARTED, + "The Codex sandbox service is running.", + ); + Ok(()) + }, + |installation, user_token| { + package_lifecycle.register_authenticated_user(installation, user_token) + }, + || { + let session = state.changed_session.swap(u32::MAX, Ordering::AcqRel); + if session == u32::MAX { + return Ok(()); + } + if let Err(error) = package_lifecycle.restore_authenticated_user(session) { + log_error( + EVENT_SERVICE_FAILED, + &format!("unable to restore package uninstall listener: {error:#}"), + ); + } + Ok(()) + }, + ) + .context("run the sandbox provisioning broker")?; + + if state.stop_requested.load(Ordering::Acquire) && state.uninstalling.load(Ordering::Acquire) { + package_lifecycle.clean_up()?; + } + Ok(()) +}