mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Canonicalize Windows sandbox homes and migrate saved junction paths (#60708)
## What changed - Canonicalize home paths before sending sandbox provisioning and desktop registration requests. - Resolve saved junction paths when restoring the sandbox owner, holding the setup lock and pinning target directories while validating existing sandbox accounts. Skip migration for retiring runtimes. - Persist the resolved home and clear desktop home ownership so uninstall cannot delete a historical junction's target. ## Testing Add a Windows junction test verifying that a saved alias resolves to its physical target and an already canonical path needs no migration. GitOrigin-RevId: 9bac672273694e5a0353fec72a6bcba120e57255
This commit is contained in:
@@ -162,7 +162,7 @@ fn provision(
|
||||
version: crate::PROVISIONING_PROTOCOL_VERSION,
|
||||
message: crate::ProvisioningMessage::ProvisionSandboxRequest {
|
||||
payload: crate::SandboxProvisioningRequest {
|
||||
codex_home: codex_home
|
||||
codex_home: crate::path_normalization::canonicalize_path(codex_home)
|
||||
.to_str()
|
||||
.context("sandbox provisioning home is not valid UTF-8")?
|
||||
.to_owned(),
|
||||
@@ -226,7 +226,7 @@ pub fn register_desktop_installation(codex_home: &Path) -> anyhow::Result<()> {
|
||||
let request = crate::FramedProvisioningMessage {
|
||||
version: crate::PROVISIONING_PROTOCOL_VERSION,
|
||||
message: crate::ProvisioningMessage::RegisterInstallationRequest {
|
||||
codex_home: codex_home
|
||||
codex_home: crate::path_normalization::canonicalize_path(codex_home)
|
||||
.to_str()
|
||||
.context("desktop home is not valid UTF-8")?
|
||||
.to_owned(),
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
use std::ffi::OsString;
|
||||
use std::io;
|
||||
use std::os::windows::ffi::OsStringExt;
|
||||
use std::os::windows::io::OwnedHandle;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::ptr;
|
||||
@@ -75,3 +76,45 @@ pub(crate) fn save(mut record: InstallationRecord) -> Result<InstallationRecord>
|
||||
save_runtime(&record)?;
|
||||
Ok(record)
|
||||
}
|
||||
|
||||
// Restore the saved owner's junction only while its physical target is pinned.
|
||||
pub(crate) fn resolve_restored_home(
|
||||
saved: &Path,
|
||||
runtime: Option<&RuntimeRegistration>,
|
||||
pins: &mut Vec<OwnedHandle>,
|
||||
) -> Result<Option<PathBuf>> {
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use windows_sys::Win32::Storage::FileSystem as fs;
|
||||
let Ok(home) = saved.canonicalize() else {
|
||||
return Ok(None);
|
||||
};
|
||||
if home == saved || runtime.is_some_and(|runtime| runtime.retiring.is_some()) {
|
||||
return Ok(None);
|
||||
}
|
||||
crate::ipc::pin_existing_ancestors(&home, pins)?;
|
||||
pins.push(crate::ipc::pin_directory(
|
||||
&home,
|
||||
fs::FILE_ADD_FILE | fs::FILE_ADD_SUBDIRECTORY | fs::WRITE_DAC,
|
||||
codex_windows_sandbox::DirectoryOpenDisposition::OpenExisting,
|
||||
)?);
|
||||
if let Some(runtime) = runtime.filter(|runtime| !runtime.accounts.is_empty()) {
|
||||
crate::ipc::pin_existing_ancestors(
|
||||
&codex_windows_sandbox::sandbox_secrets_dir(&home),
|
||||
pins,
|
||||
)?;
|
||||
for entry in &runtime.accounts {
|
||||
let token =
|
||||
codex_windows_sandbox::logon_existing_sandbox_account(&home, entry.account)?;
|
||||
crate::registered_runtime::validate_target(
|
||||
token.as_raw_handle(),
|
||||
entry.account,
|
||||
Some(&entry.user_sid),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
Ok(Some(home))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "installation_record_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
#![cfg(test)]
|
||||
//! Covers registration of an existing Windows home through its directory junction.
|
||||
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn only_the_saved_junction_needs_migration() -> Result<()> {
|
||||
let dir = tempfile::tempdir()?;
|
||||
let target = tempfile::tempdir_in(dir.path())?;
|
||||
let alias = dir.path().join("alias");
|
||||
let status = std::process::Command::new("cmd.exe")
|
||||
.args(["/d", "/c", "mklink", "/j"])
|
||||
.arg(&alias)
|
||||
.arg(target.path())
|
||||
.status()?;
|
||||
anyhow::ensure!(status.success(), "create test junction");
|
||||
let physical = target.path().canonicalize()?;
|
||||
let resolve = |saved: &Path| {
|
||||
resolve_restored_home(saved, /*runtime*/ None, &mut Vec::new())
|
||||
};
|
||||
assert_eq!(resolve(&alias)?, Some(physical.clone()));
|
||||
assert_eq!(resolve(&physical)?, None);
|
||||
Ok(())
|
||||
}
|
||||
@@ -223,7 +223,9 @@ impl PackageLifecycle {
|
||||
if self.installation.borrow().is_some() {
|
||||
return Ok(());
|
||||
}
|
||||
let Some(record) = crate::installation_record::load()? else {
|
||||
let setup_lock =
|
||||
codex_windows_sandbox::acquire_sandbox_setup_lock(/*timeout_ms*/ 5_000)?;
|
||||
let Some(mut record) = crate::installation_record::load()? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
@@ -245,6 +247,23 @@ impl PackageLifecycle {
|
||||
"logged-in user does not match the recorded sandbox owner"
|
||||
);
|
||||
|
||||
let mut pins = Vec::new();
|
||||
if let Some(home) = with_owner_impersonation(token.as_raw_handle(), || {
|
||||
crate::installation_record::resolve_restored_home(
|
||||
&record.codex_home,
|
||||
record.runtime.as_ref(),
|
||||
&mut pins,
|
||||
)
|
||||
})? {
|
||||
record.codex_home = home;
|
||||
if let Some(desktop) = &mut record.desktop_installation {
|
||||
// A historical junction cannot authorize deleting its target on uninstall.
|
||||
desktop.created_codex_home = false;
|
||||
}
|
||||
crate::installation_record::save_runtime(&record)?;
|
||||
}
|
||||
drop(setup_lock);
|
||||
|
||||
self.register_authenticated_user(
|
||||
InstallationRecord {
|
||||
session_id,
|
||||
|
||||
Reference in New Issue
Block a user