mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Clarify registered Windows sandbox setup errors (#47673)
## Why Incomplete setup for the installed app reported the same error as an owner mismatch or removal in progress, obscuring the need to retry sandbox setup. ## What changed - Distinguish owner mismatch, Codex home mismatch, removal in progress, and incomplete setup when authorizing the registered runner receipt. - Tell users to retry sandbox setup when the receipt is not ready for the installed app. - Add operation and account context to provisioning failures, including account logon, profile preparation, package queries, alias resolution, and registration persistence. ## Testing Add a unit test covering distinct authorization errors and their precedence, acceptance of a ready receipt, and rejection after the app package changes. GitOrigin-RevId: 4e7620c4670e8b02a411a176cc89bd72dcd6e478
This commit is contained in:
@@ -196,13 +196,8 @@ pub(crate) fn registered_runner_alias(
|
||||
let package =
|
||||
current_package_full_name()?.context("registered Core launch requires an installed app")?;
|
||||
let owner = crate::winutil::resolve_sid(&crate::runtime_ownership::current_setup_user()?)?;
|
||||
anyhow::ensure!(
|
||||
record.user_sid
|
||||
== crate::winutil::string_from_sid_bytes(&owner).map_err(anyhow::Error::msg)?
|
||||
&& record.codex_home == codex_home.canonicalize()?
|
||||
&& record.runtime()?.ready_for_package(&package),
|
||||
"registered Core setup belongs to another owner or is being removed"
|
||||
);
|
||||
let owner = crate::winutil::string_from_sid_bytes(&owner).map_err(anyhow::Error::msg)?;
|
||||
authorize_runner_receipt(&record, &owner, &codex_home.canonicalize()?, &package)?;
|
||||
let entry = record
|
||||
.runtime()?
|
||||
.accounts
|
||||
@@ -227,6 +222,32 @@ pub(crate) fn registered_runner_alias(
|
||||
Ok(path.clone())
|
||||
}
|
||||
|
||||
fn authorize_runner_receipt(
|
||||
record: &crate::installation_record::InstallationRecord,
|
||||
owner: &str,
|
||||
codex_home: &Path,
|
||||
package: &str,
|
||||
) -> Result<()> {
|
||||
ensure!(
|
||||
record.user_sid == owner,
|
||||
"registered Core setup belongs to another owner"
|
||||
);
|
||||
ensure!(
|
||||
record.codex_home == codex_home,
|
||||
"registered Core setup belongs to another Codex home"
|
||||
);
|
||||
let runtime = record.runtime()?;
|
||||
ensure!(
|
||||
runtime.retiring.is_none(),
|
||||
"registered Core setup is being removed"
|
||||
);
|
||||
ensure!(
|
||||
runtime.ready_for_package(package),
|
||||
"registered Core setup is not complete for the installed app; retry sandbox setup"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "app_package_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,11 +1,17 @@
|
||||
//! Runtime opt-in and bounded OS package-name results, without installed-state changes.
|
||||
|
||||
use super::authorize_runner_receipt;
|
||||
use super::query_package_name;
|
||||
use super::requested_value;
|
||||
use crate::installation_record::InstallationRecord;
|
||||
use crate::runtime_ownership::RuntimeAccountRegistration;
|
||||
use crate::runtime_ownership::RuntimeRegistration;
|
||||
use crate::runtime_ownership::SandboxRuntimeAccount;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::ffi::OsStr;
|
||||
use std::ffi::OsString;
|
||||
use std::os::windows::ffi::OsStringExt;
|
||||
use std::path::Path;
|
||||
use windows_sys::Win32::Foundation::APPMODEL_ERROR_NO_PACKAGE;
|
||||
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
|
||||
use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
|
||||
@@ -29,6 +35,78 @@ fn only_exact_opt_in_requests_registered_core() {
|
||||
assert!(!requested_value(Some(&OsString::from_wide(&[0xd800]))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn runner_receipt_distinguishes_incomplete_setup_from_owner_and_removal() {
|
||||
let owner = "S-1-5-21-100";
|
||||
let home = Path::new(r"C:\Users\owner\.codex");
|
||||
let package = "OpenAI.Codex_current";
|
||||
let runtime = RuntimeRegistration {
|
||||
package_family: "OpenAI.Codex_family".to_owned(),
|
||||
accounts: Vec::new(),
|
||||
metadata_roots: Vec::new(),
|
||||
ready_package: None,
|
||||
retiring: None,
|
||||
};
|
||||
let mut record = InstallationRecord {
|
||||
user_sid: owner.to_owned(),
|
||||
codex_home: home.to_owned(),
|
||||
session_id: 1,
|
||||
desktop_installation: None,
|
||||
runtime: Some(runtime),
|
||||
};
|
||||
let incomplete =
|
||||
"registered Core setup is not complete for the installed app; retry sandbox setup";
|
||||
assert_eq!(
|
||||
authorize_runner_receipt(&record, owner, home, package)
|
||||
.unwrap_err()
|
||||
.to_string(),
|
||||
incomplete
|
||||
);
|
||||
record.runtime.as_mut().unwrap().retiring = Some("removing".to_owned());
|
||||
for (candidate_owner, candidate_home, message) in [
|
||||
(
|
||||
"S-1-5-21-200",
|
||||
home,
|
||||
"registered Core setup belongs to another owner",
|
||||
),
|
||||
(
|
||||
owner,
|
||||
Path::new(r"C:\Users\other\.codex"),
|
||||
"registered Core setup belongs to another Codex home",
|
||||
),
|
||||
(owner, home, "registered Core setup is being removed"),
|
||||
] {
|
||||
assert_eq!(
|
||||
authorize_runner_receipt(&record, candidate_owner, candidate_home, package)
|
||||
.unwrap_err()
|
||||
.to_string(),
|
||||
message
|
||||
);
|
||||
}
|
||||
let runtime = record.runtime.as_mut().unwrap();
|
||||
runtime.retiring = None;
|
||||
runtime.accounts = [
|
||||
SandboxRuntimeAccount::Offline,
|
||||
SandboxRuntimeAccount::Online,
|
||||
]
|
||||
.into_iter()
|
||||
.map(|account| RuntimeAccountRegistration {
|
||||
cleanup_logon_pending: false,
|
||||
account,
|
||||
user_sid: format!("{account:?}"),
|
||||
alias_path: Some(home.join(account.username())),
|
||||
})
|
||||
.collect();
|
||||
runtime.ready_package = Some(package.to_owned());
|
||||
assert!(authorize_runner_receipt(&record, owner, home, package).is_ok());
|
||||
assert_eq!(
|
||||
authorize_runner_receipt(&record, owner, home, "OpenAI.Codex_updated")
|
||||
.unwrap_err()
|
||||
.to_string(),
|
||||
incomplete
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn package_query_distinguishes_absence_from_failure_and_bounds_allocation() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -77,13 +77,15 @@ pub(super) fn run(
|
||||
retiring: None,
|
||||
});
|
||||
// Persist the authenticated owner before setup can rotate shared credentials.
|
||||
crate::installation_record::save_runtime(&installation)?;
|
||||
crate::installation_record::save_runtime(&installation)
|
||||
.context("persist registered sandbox owner")?;
|
||||
// Expired passwords require full setup to rotate and persist credentials before logon.
|
||||
for account in [
|
||||
codex_windows_sandbox::OFFLINE_USERNAME,
|
||||
codex_windows_sandbox::ONLINE_USERNAME,
|
||||
] {
|
||||
setup_complete &= codex_windows_sandbox::local_user_flags(account)?
|
||||
setup_complete &= codex_windows_sandbox::local_user_flags(account)
|
||||
.with_context(|| format!("inspect registered sandbox account {account}"))?
|
||||
.is_some_and(|flags| flags & (UF_ACCOUNTDISABLE | UF_PASSWORD_EXPIRED) == 0);
|
||||
}
|
||||
if !setup_complete {
|
||||
@@ -93,7 +95,8 @@ pub(super) fn run(
|
||||
"registration refresh cannot repair sandbox setup"
|
||||
);
|
||||
// Setup resolves this name for ACL trustees; it must still name the held token's user.
|
||||
let setup_owner_sid = codex_windows_sandbox::resolve_sid(&identity.account)?;
|
||||
let setup_owner_sid = codex_windows_sandbox::resolve_sid(&identity.account)
|
||||
.context("resolve registered sandbox setup owner")?;
|
||||
ensure!(
|
||||
string_from_sid_bytes(&setup_owner_sid).map_err(anyhow::Error::msg)?
|
||||
== identity.user_sid,
|
||||
@@ -104,14 +107,18 @@ pub(super) fn run(
|
||||
&identity.account,
|
||||
settings,
|
||||
identity.runtime,
|
||||
)?;
|
||||
)
|
||||
.context("provision registered sandbox accounts and permissions")?;
|
||||
}
|
||||
crate::registered_runtime::provision(
|
||||
identity,
|
||||
&mut installation,
|
||||
&package.FullName()?,
|
||||
&package
|
||||
.FullName()
|
||||
.context("read registered sandbox package name")?,
|
||||
shutdown,
|
||||
)
|
||||
.context("prepare registered sandbox runtime")
|
||||
})();
|
||||
result
|
||||
.inspect_err(|error| {
|
||||
|
||||
@@ -106,7 +106,8 @@ pub(crate) fn provision(
|
||||
&mut pins,
|
||||
)?;
|
||||
logon_existing_sandbox_account(&identity.codex_home, account)
|
||||
})?;
|
||||
})
|
||||
.with_context(|| format!("log on managed runtime account {account:?}"))?;
|
||||
let expected = record
|
||||
.runtime()?
|
||||
.accounts
|
||||
@@ -116,8 +117,10 @@ pub(crate) fn provision(
|
||||
token,
|
||||
account,
|
||||
expected.map(|entry| entry.user_sid.as_str()),
|
||||
)?;
|
||||
let installed = registered_packages(&profile.user_sid, &family)?;
|
||||
)
|
||||
.with_context(|| format!("prepare managed runtime profile for {account:?}"))?;
|
||||
let installed = registered_packages(&profile.user_sid, &family)
|
||||
.with_context(|| format!("query existing runtime packages for {account:?}"))?;
|
||||
if expected.is_none() {
|
||||
ensure!(
|
||||
installed.is_empty(),
|
||||
@@ -128,7 +131,8 @@ pub(crate) fn provision(
|
||||
profile.token.as_raw_handle() as _,
|
||||
&FOLDERID_LocalAppData,
|
||||
/*flags*/ 0,
|
||||
)?
|
||||
)
|
||||
.with_context(|| format!("resolve runtime alias location for {account:?}"))?
|
||||
.join(r"Microsoft\WindowsApps")
|
||||
.join(&family)
|
||||
.join(APP_CORE_RUNNER_ALIAS);
|
||||
@@ -148,7 +152,8 @@ pub(crate) fn provision(
|
||||
record.runtime_mut()?.accounts = accounts;
|
||||
if !receipt_current {
|
||||
record.runtime_mut()?.ready_package = None;
|
||||
crate::installation_record::save_runtime(record)?;
|
||||
crate::installation_record::save_runtime(record)
|
||||
.context("persist managed runtime account registration")?;
|
||||
}
|
||||
for (mut profile, installed) in profiles {
|
||||
if !installed {
|
||||
|
||||
Reference in New Issue
Block a user