mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Prefer the provisioning service for automatic Windows sandbox setup (#46239)
## What changed - Use the installed provisioning service regardless of the onboarding feature gate. Fall back to the elevated helper only when the service is unavailable; propagate service errors. - Pass the effective proxy settings and filter listener ports to match them, including when preserving saved settings. - Remove stale credentials before repairing missing, disabled, or password-expired sandbox accounts so older services cannot mistake them for completed setup. - Select helper fallback when workload-identity environment variables are present, since service requests do not carry that environment. Continue rejecting helper fallback for registered Core. ## Testing Extend account-repair tests to cover stale credential removal, missing and disabled accounts, and repeated setup checks. Add subprocess tests for workload-identity routing with and without registered Core. GitOrigin-RevId: 1041ae76e9b0893ad60f30a5cdc506748be52552
This commit is contained in:
@@ -1,4 +1,7 @@
|
||||
use crate::SandboxRuntimeAccount;
|
||||
use crate::WindowsSandboxProvisioningOutcome;
|
||||
use crate::WindowsSandboxProvisioningSettings;
|
||||
use crate::WindowsSandboxProxyListeners;
|
||||
use crate::dpapi;
|
||||
use crate::logging::debug_log;
|
||||
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
|
||||
@@ -286,7 +289,7 @@ pub(crate) fn require_sandbox_account(
|
||||
require_sandbox_account_with_setup(
|
||||
request,
|
||||
proxy_settings_mode,
|
||||
run_elevated_setup_with_proxy_settings,
|
||||
run_automatic_setup,
|
||||
local_user_flags,
|
||||
)
|
||||
}
|
||||
@@ -349,8 +352,10 @@ fn require_sandbox_account_with_setup(
|
||||
Err(_) => false,
|
||||
};
|
||||
if needs_repair {
|
||||
setup_reason =
|
||||
Some("sandbox account is missing, disabled, or password expired".to_string());
|
||||
let reason = "sandbox account is missing, disabled, or password expired";
|
||||
// Older services trust these credentials as proof of completed setup.
|
||||
remove_sandbox_users_file(codex_home, reason)?;
|
||||
setup_reason = Some(reason.to_string());
|
||||
identity = None;
|
||||
break;
|
||||
}
|
||||
@@ -400,6 +405,35 @@ fn require_sandbox_account_with_setup(
|
||||
))
|
||||
}
|
||||
|
||||
// Automatic setup prefers an installed service regardless of the onboarding feature gate.
|
||||
// Only an unavailable service may fall back to the UAC helper; service errors propagate.
|
||||
fn run_automatic_setup(
|
||||
request: SandboxSetupRequest<'_>,
|
||||
settings: &OfflineProxySettings,
|
||||
) -> Result<()> {
|
||||
let mut listeners = WindowsSandboxProxyListeners::from_proxy_environment(request.env_map);
|
||||
// Preserve-mode setup can use saved ports that differ from the current environment.
|
||||
listeners
|
||||
.http_ports
|
||||
.retain(|port| settings.proxy_ports.contains(port));
|
||||
listeners
|
||||
.socks_ports
|
||||
.retain(|port| settings.proxy_ports.contains(port));
|
||||
match crate::provision_windows_sandbox_via_service(
|
||||
request.codex_home,
|
||||
WindowsSandboxProvisioningSettings {
|
||||
proxy_ports: settings.proxy_ports.clone(),
|
||||
allow_local_binding: settings.allow_local_binding,
|
||||
},
|
||||
listeners,
|
||||
)? {
|
||||
WindowsSandboxProvisioningOutcome::Provisioned => Ok(()),
|
||||
WindowsSandboxProvisioningOutcome::Unavailable => {
|
||||
run_elevated_setup_with_proxy_settings(request, settings)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn desired_offline_proxy_settings(
|
||||
marker: Option<&SetupMarker>,
|
||||
proxy_settings_mode: crate::WindowsSandboxProxySettingsMode,
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
//! or helper launches.
|
||||
|
||||
use super::require_sandbox_account_with_setup;
|
||||
use super::sandbox_setup_is_complete_with_settings;
|
||||
use crate::WindowsSandboxProvisioningSettings;
|
||||
use crate::WindowsSandboxProxySettingsMode;
|
||||
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
|
||||
use crate::setup::OFFLINE_USERNAME;
|
||||
@@ -23,6 +25,7 @@ use pretty_assertions::assert_eq;
|
||||
use std::cell::Cell;
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_NORMAL_ACCOUNT;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
|
||||
@@ -77,6 +80,8 @@ fn credential_setup_repairs_expired_accounts_once_and_reloads_credentials() -> R
|
||||
},
|
||||
WindowsSandboxProxySettingsMode::Preserve,
|
||||
|_, _| {
|
||||
// Older services must not accept stale credentials instead of repairing expiry.
|
||||
assert!(!sandbox_users_path(home.path()).exists());
|
||||
setups.set(setups.get() + 1);
|
||||
users.offline.password =
|
||||
BASE64.encode(crate::dpapi::protect(b"new-test-password")?);
|
||||
@@ -119,18 +124,22 @@ fn credential_setup_repairs_expired_accounts_once_and_reloads_credentials() -> R
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn credential_setup_reconciles_effective_firewall_policy() -> Result<()> {
|
||||
fn credential_setup_reconciles_policy_and_repairs_accounts() -> Result<()> {
|
||||
let permissions = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&PermissionProfile::read_only(),
|
||||
)?;
|
||||
for (stored_binding, desired_binding, ports, expected_full_setups) in [
|
||||
(true, true, "8080", 0),
|
||||
(true, true, "", 0),
|
||||
(true, true, "8080,3129", 0),
|
||||
(false, false, "8080", 1),
|
||||
(false, false, "", 1),
|
||||
(false, true, "3128", 1),
|
||||
(true, false, "3128", 1),
|
||||
let enabled = Some(UF_NORMAL_ACCOUNT);
|
||||
let disabled = Some(UF_NORMAL_ACCOUNT | UF_ACCOUNTDISABLE);
|
||||
for (stored_binding, desired_binding, ports, account_flags, expected_full_setups) in [
|
||||
(true, true, "8080", enabled, 0),
|
||||
(true, true, "", enabled, 0),
|
||||
(true, true, "8080,3129", enabled, 0),
|
||||
(false, false, "8080", enabled, 1),
|
||||
(false, false, "", enabled, 1),
|
||||
(false, true, "3128", enabled, 1),
|
||||
(true, false, "3128", enabled, 1),
|
||||
(true, true, "3128", None, 1),
|
||||
(true, true, "3128", disabled, 1),
|
||||
] {
|
||||
let full_setups = Cell::new(/*value*/ 0);
|
||||
let home = tempfile::tempdir()?;
|
||||
@@ -169,6 +178,7 @@ fn credential_setup_reconciles_effective_firewall_policy() -> Result<()> {
|
||||
u8::from(desired_binding).to_string(),
|
||||
),
|
||||
]);
|
||||
let account_flags = Cell::new(account_flags);
|
||||
let prepare = || {
|
||||
require_sandbox_account_with_setup(
|
||||
&SandboxSetupRequest {
|
||||
@@ -179,7 +189,15 @@ fn credential_setup_reconciles_effective_firewall_policy() -> Result<()> {
|
||||
proxy_enforced: true,
|
||||
},
|
||||
WindowsSandboxProxySettingsMode::Reconcile,
|
||||
|_, desired| {
|
||||
|request, desired| {
|
||||
// Matching artifacts must not let setup skip account repair.
|
||||
assert!(!sandbox_setup_is_complete_with_settings(
|
||||
request.codex_home,
|
||||
&WindowsSandboxProvisioningSettings {
|
||||
proxy_ports: desired.proxy_ports.clone(),
|
||||
allow_local_binding: desired.allow_local_binding,
|
||||
}
|
||||
));
|
||||
full_setups.set(full_setups.get() + 1);
|
||||
let mut reconciled = marker.clone();
|
||||
reconciled.proxy_ports = desired.proxy_ports.clone();
|
||||
@@ -188,9 +206,11 @@ fn credential_setup_reconciles_effective_firewall_policy() -> Result<()> {
|
||||
setup_marker_path(home.path()),
|
||||
serde_json::to_vec(&reconciled)?,
|
||||
)?;
|
||||
fs::write(sandbox_users_path(home.path()), serde_json::to_vec(&users)?)?;
|
||||
account_flags.set(enabled);
|
||||
Ok(())
|
||||
},
|
||||
|_| Ok(Some(UF_NORMAL_ACCOUNT)),
|
||||
|_| Ok(account_flags.get()),
|
||||
)
|
||||
};
|
||||
for _ in 0..2 {
|
||||
|
||||
@@ -18,6 +18,8 @@ use anyhow::Context;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::bail;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR;
|
||||
use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR;
|
||||
use windows_sys::Win32::Foundation::ERROR_BROKEN_PIPE;
|
||||
use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND;
|
||||
use windows_sys::Win32::Foundation::ERROR_NO_DATA;
|
||||
@@ -135,6 +137,14 @@ pub fn provision_windows_sandbox_via_service(
|
||||
settings: WindowsSandboxProvisioningSettings,
|
||||
listeners: WindowsSandboxProxyListeners,
|
||||
) -> anyhow::Result<WindowsSandboxProvisioningOutcome> {
|
||||
// The IPC request does not carry the caller's workload-identity environment.
|
||||
// Select helper fallback here: a service error would propagate, not fall back.
|
||||
// service_unavailable still rejects helper fallback for registered Core.
|
||||
if std::env::var_os(OPENAI_FEDERATION_RULE_ID_ENV_VAR).is_some()
|
||||
|| std::env::var_os(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR).is_some()
|
||||
{
|
||||
return service_unavailable();
|
||||
}
|
||||
provision(codex_home, settings, listeners, ProvisioningIntent::Setup)
|
||||
}
|
||||
|
||||
@@ -385,3 +395,7 @@ impl Drop for ServiceHandle {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "provisioning_client_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
70
codex-rs/windows-sandbox-rs/src/provisioning_client_tests.rs
Normal file
70
codex-rs/windows-sandbox-rs/src/provisioning_client_tests.rs
Normal file
@@ -0,0 +1,70 @@
|
||||
//! Exercises process-local workload-identity routing without contacting the service.
|
||||
|
||||
use super::WindowsSandboxProvisioningOutcome;
|
||||
use super::provision_windows_sandbox_via_service;
|
||||
use crate::WindowsSandboxProvisioningSettings;
|
||||
use crate::WindowsSandboxProxyListeners;
|
||||
use anyhow::Result;
|
||||
use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR;
|
||||
use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::ffi::OsString;
|
||||
use std::os::windows::ffi::OsStringExt;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
|
||||
#[test]
|
||||
fn workload_identity_selects_helper_fallback_except_for_registered_core() -> Result<()> {
|
||||
for variable in [
|
||||
OPENAI_FEDERATION_RULE_ID_ENV_VAR,
|
||||
OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR,
|
||||
] {
|
||||
for registered_core in ["0", "1"] {
|
||||
let output = Command::new(std::env::current_exe()?)
|
||||
.args([
|
||||
"--exact",
|
||||
"provisioning_client::tests::workload_identity_child",
|
||||
"--ignored",
|
||||
"--nocapture",
|
||||
])
|
||||
.env_remove(OPENAI_FEDERATION_RULE_ID_ENV_VAR)
|
||||
.env_remove(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR)
|
||||
.env(variable, "test-only")
|
||||
.env("CODEX_WINDOWS_REGISTERED_CORE", registered_core)
|
||||
.output()?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"{variable}, registered_core={registered_core}: {}{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "child process for workload_identity_selects_helper_fallback_except_for_registered_core"]
|
||||
fn workload_identity_child() -> Result<()> {
|
||||
// If the early guard regresses, this fails payload validation before any IPC.
|
||||
let invalid_home = PathBuf::from(OsString::from_wide(&[0xd800]));
|
||||
let outcome = provision_windows_sandbox_via_service(
|
||||
&invalid_home,
|
||||
WindowsSandboxProvisioningSettings {
|
||||
proxy_ports: Vec::new(),
|
||||
allow_local_binding: false,
|
||||
},
|
||||
WindowsSandboxProxyListeners::default(),
|
||||
);
|
||||
if crate::registered_core_requested() {
|
||||
assert_eq!(
|
||||
outcome
|
||||
.expect_err("registered Core must not fall back")
|
||||
.to_string(),
|
||||
"app runtime provisioning service is unavailable; refusing helper fallback"
|
||||
);
|
||||
} else {
|
||||
assert_eq!(outcome?, WindowsSandboxProvisioningOutcome::Unavailable);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user