mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Skip managed config loading for registered Windows sandbox refreshes (#50480)
## Why Registration-only refreshes retain the existing sandbox, so package maintenance should not require another cloud-policy fetch. ## What changed Skip managed configuration loading when a provisioning request has both `registered_core` and `refresh_only` set. The registered-runtime transaction still checks the existing owner, accounts, and unchanged settings under the setup lock. Other provisioning requests continue to enforce managed policy. ## Testing Update the invalid-impersonation-token test to cover all four flag combinations, verifying that only registered refreshes succeed without loading configuration. GitOrigin-RevId: e65ba1359be65e6314fa483b24fb66ed57a444ac
This commit is contained in:
@@ -57,10 +57,9 @@ pub(super) fn authenticate_client(
|
||||
// Registration does not provision resources or change sandbox policy.
|
||||
ServiceRequest::RegisterInstallation { .. } => Ok(()),
|
||||
ServiceRequest::ProvisionSandbox(request) => {
|
||||
crate::machine_policy::validate_provisioning_settings(
|
||||
crate::machine_policy::validate_provisioning_request(
|
||||
&identity.codex_home,
|
||||
&request.settings,
|
||||
&request.listeners,
|
||||
request,
|
||||
identity.token.as_raw_handle(),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
//! Validates provisioning requests against the standard managed configuration layers.
|
||||
//! Validates sandbox provisioning against managed policy; registration-only refresh
|
||||
//! retains the existing sandbox and is admitted by the registered-runtime transaction.
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
@@ -17,6 +18,8 @@ use tokio::sync::Notify;
|
||||
use windows_sys::Win32::Foundation::HANDLE;
|
||||
use windows_sys::Win32::Security::ImpersonateLoggedOnUser;
|
||||
|
||||
use crate::ipc::ProvisioningRequest;
|
||||
|
||||
struct NonOwningImpersonationToken(HANDLE);
|
||||
|
||||
// SAFETY: Windows access token handles are opaque process-wide values that may
|
||||
@@ -25,12 +28,18 @@ struct NonOwningImpersonationToken(HANDLE);
|
||||
unsafe impl Send for NonOwningImpersonationToken {}
|
||||
unsafe impl Sync for NonOwningImpersonationToken {}
|
||||
|
||||
pub(crate) fn validate_provisioning_settings(
|
||||
pub(crate) fn validate_provisioning_request(
|
||||
codex_home: &Path,
|
||||
settings: &WindowsSandboxProvisioningSettings,
|
||||
listeners: &WindowsSandboxProxyListeners,
|
||||
request: &ProvisioningRequest,
|
||||
impersonation_token: HANDLE,
|
||||
) -> Result<()> {
|
||||
if request.registered_core && request.refresh_only {
|
||||
// The caller is already authenticated. registered::run checks the existing
|
||||
// owner, accounts and unchanged settings under the setup lock; it cannot
|
||||
// provision or repair a sandbox. Backend use still obeys managed config.
|
||||
// Package maintenance must not require another cloud-policy fetch.
|
||||
return Ok(());
|
||||
}
|
||||
let impersonation_failure = Arc::new(Notify::new());
|
||||
let worker_impersonation_failure = Arc::clone(&impersonation_failure);
|
||||
let impersonation_token = NonOwningImpersonationToken(impersonation_token);
|
||||
@@ -90,7 +99,7 @@ pub(crate) fn validate_provisioning_settings(
|
||||
}
|
||||
})?;
|
||||
|
||||
validate_requirements(settings, listeners, &requirements)
|
||||
validate_requirements(&request.settings, &request.listeners, &requirements)
|
||||
.context("enforce managed provisioning requirements")
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,9 @@ use anyhow::Result;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_windows_sandbox::WindowsSandboxProvisioningSettings;
|
||||
use codex_windows_sandbox::WindowsSandboxProxyListeners;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use crate::ipc::ProvisioningRequest;
|
||||
|
||||
fn validate_requirements(
|
||||
settings: &WindowsSandboxProvisioningSettings,
|
||||
@@ -13,16 +16,29 @@ fn validate_requirements(
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn runtime_worker_impersonation_failure_rejects_provisioning() {
|
||||
let error = super::validate_provisioning_settings(
|
||||
&std::env::temp_dir(),
|
||||
&WindowsSandboxProvisioningSettings::default(),
|
||||
&WindowsSandboxProxyListeners::default(),
|
||||
std::ptr::null_mut(),
|
||||
)
|
||||
.expect_err("runtime workers must not load configuration without impersonating the client");
|
||||
|
||||
assert!(error.to_string().contains("failed to impersonate"));
|
||||
fn only_registered_refresh_skips_configuration_loading() {
|
||||
for (registered_core, refresh_only) in
|
||||
[(true, true), (true, false), (false, false), (false, true)]
|
||||
{
|
||||
let request = ProvisioningRequest {
|
||||
codex_home: std::env::temp_dir(),
|
||||
registered_core,
|
||||
refresh_only,
|
||||
settings: WindowsSandboxProvisioningSettings::default(),
|
||||
listeners: WindowsSandboxProxyListeners::default(),
|
||||
};
|
||||
// An invalid worker token makes any attempt to load config fail closed.
|
||||
// Refresh must succeed without starting that runtime; setup must not.
|
||||
let result = super::validate_provisioning_request(
|
||||
&request.codex_home,
|
||||
&request,
|
||||
std::ptr::null_mut(),
|
||||
);
|
||||
assert_eq!(result.is_ok(), registered_core && refresh_only);
|
||||
if let Err(error) = result {
|
||||
assert!(error.to_string().contains("failed to impersonate"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user