mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Separate Windows sandbox provisioning from ACL refresh (#42309)
## What changed - Limit elevated interactive setup to provisioning sandbox identities and proxy settings; do not include caller workspace or ACL roots in its payload. - Always run the non-elevated setup refresh during sandbox activation so the current permission profile supplies read, write, and deny paths, including when provisioning was completed previously. - Add an `interactive-provision` setup mode that performs provisioning without the full ACL setup path. ## Testing - Verify elevated provisioning payloads contain no caller ACL roots and use the new mode. - Verify the setup helper accepts `interactive-provision` payloads. GitOrigin-RevId: 081b1b43abd2388773c230fb35da82896e7be2ea
This commit is contained in:
@@ -99,27 +99,34 @@ pub fn sandbox_setup_is_complete(_codex_home: &Path) -> bool {
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub fn run_elevated_setup(
|
||||
pub fn prepare_elevated_sandbox(
|
||||
permission_profile: &PermissionProfile,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
command_cwd: &Path,
|
||||
env_map: &HashMap<String, String>,
|
||||
codex_home: &Path,
|
||||
) -> anyhow::Result<()> {
|
||||
let permissions =
|
||||
codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
)?;
|
||||
codex_windows_sandbox::run_elevated_setup(
|
||||
codex_windows_sandbox::SandboxSetupRequest {
|
||||
if !sandbox_setup_is_complete(codex_home) {
|
||||
let permissions =
|
||||
codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
)?;
|
||||
codex_windows_sandbox::run_elevated_setup(codex_windows_sandbox::SandboxSetupRequest {
|
||||
permissions: &permissions,
|
||||
command_cwd,
|
||||
env_map,
|
||||
codex_home,
|
||||
proxy_enforced: false,
|
||||
},
|
||||
codex_windows_sandbox::SetupRootOverrides::default(),
|
||||
})?;
|
||||
}
|
||||
codex_windows_sandbox::run_setup_refresh(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
command_cwd,
|
||||
env_map,
|
||||
codex_home,
|
||||
/*proxy_enforced*/ false,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -150,7 +157,7 @@ pub fn run_elevated_provisioning_setup(
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub fn run_elevated_setup(
|
||||
pub fn prepare_elevated_sandbox(
|
||||
_permission_profile: &PermissionProfile,
|
||||
_workspace_roots: &[AbsolutePathBuf],
|
||||
_command_cwd: &Path,
|
||||
@@ -286,15 +293,13 @@ async fn run_windows_sandbox_setup_and_persist(
|
||||
let setup_result = tokio::task::spawn_blocking(move || -> anyhow::Result<()> {
|
||||
match mode {
|
||||
WindowsSandboxSetupMode::Elevated => {
|
||||
if !sandbox_setup_is_complete(setup_codex_home.as_path()) {
|
||||
run_elevated_setup(
|
||||
&permission_profile,
|
||||
workspace_roots.as_slice(),
|
||||
command_cwd.as_path(),
|
||||
&env_map,
|
||||
setup_codex_home.as_path(),
|
||||
)?;
|
||||
}
|
||||
prepare_elevated_sandbox(
|
||||
&permission_profile,
|
||||
workspace_roots.as_slice(),
|
||||
command_cwd.as_path(),
|
||||
&env_map,
|
||||
setup_codex_home.as_path(),
|
||||
)?;
|
||||
}
|
||||
WindowsSandboxSetupMode::Unelevated => {
|
||||
run_legacy_setup_preflight(
|
||||
|
||||
@@ -1780,22 +1780,11 @@ impl App {
|
||||
let codex_home = self.config.codex_home.clone();
|
||||
let tx = self.app_event_tx.clone();
|
||||
|
||||
// If the elevated setup already ran on this machine, don't prompt for
|
||||
// elevation again - just flip the config to use the elevated path.
|
||||
if crate::windows_sandbox::sandbox_setup_is_complete(codex_home.as_path()) {
|
||||
tx.send(AppEvent::EnableWindowsSandboxForAgentMode {
|
||||
preset,
|
||||
mode: WindowsSandboxEnableMode::Elevated,
|
||||
profile_selection,
|
||||
});
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
self.chat_widget.show_windows_sandbox_setup_status();
|
||||
self.windows_sandbox.setup_started_at = Some(Instant::now());
|
||||
let session_telemetry = self.session_telemetry.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
let result = crate::windows_sandbox::run_elevated_setup(
|
||||
let result = crate::windows_sandbox::prepare_elevated_sandbox(
|
||||
&permission_profile,
|
||||
workspace_roots.as_slice(),
|
||||
command_cwd.as_path(),
|
||||
|
||||
@@ -61,26 +61,33 @@ pub(crate) fn sandbox_setup_is_complete(_codex_home: &Path) -> bool {
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub(crate) fn run_elevated_setup(
|
||||
pub(crate) fn prepare_elevated_sandbox(
|
||||
permission_profile: &PermissionProfile,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
command_cwd: &Path,
|
||||
env_map: &HashMap<String, String>,
|
||||
codex_home: &Path,
|
||||
) -> anyhow::Result<()> {
|
||||
let permissions = codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
)?;
|
||||
codex_windows_sandbox::run_elevated_setup(
|
||||
codex_windows_sandbox::SandboxSetupRequest {
|
||||
if !sandbox_setup_is_complete(codex_home) {
|
||||
let permissions = codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
)?;
|
||||
codex_windows_sandbox::run_elevated_setup(codex_windows_sandbox::SandboxSetupRequest {
|
||||
permissions: &permissions,
|
||||
command_cwd,
|
||||
env_map,
|
||||
codex_home,
|
||||
proxy_enforced: false,
|
||||
},
|
||||
codex_windows_sandbox::SetupRootOverrides::default(),
|
||||
})?;
|
||||
}
|
||||
codex_windows_sandbox::run_setup_refresh(
|
||||
permission_profile,
|
||||
workspace_roots,
|
||||
command_cwd,
|
||||
env_map,
|
||||
codex_home,
|
||||
/*proxy_enforced*/ false,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -112,6 +112,7 @@ struct Payload {
|
||||
enum SetupMode {
|
||||
#[default]
|
||||
Full,
|
||||
InteractiveProvision,
|
||||
ProvisionOnly,
|
||||
ReadAclsOnly,
|
||||
}
|
||||
@@ -304,9 +305,9 @@ fn lock_sandbox_dir(
|
||||
setup_mode: SetupMode,
|
||||
) -> Result<()> {
|
||||
// ProvisionOnly accepts another user's CODEX_HOME; keep its ACL mutation
|
||||
// bound to a no-reparse handle without changing full setup behavior.
|
||||
// bound to a no-reparse handle without changing interactive setup behavior.
|
||||
let directory = match setup_mode {
|
||||
SetupMode::Full | SetupMode::ReadAclsOnly => {
|
||||
SetupMode::Full | SetupMode::InteractiveProvision | SetupMode::ReadAclsOnly => {
|
||||
std::fs::create_dir_all(dir)?;
|
||||
None
|
||||
}
|
||||
@@ -513,7 +514,9 @@ fn run_setup(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Result<(
|
||||
}
|
||||
match payload.mode {
|
||||
SetupMode::ReadAclsOnly => run_read_acl_only(payload, log),
|
||||
SetupMode::ProvisionOnly => run_provision_only(payload, log, sbx_dir),
|
||||
SetupMode::InteractiveProvision | SetupMode::ProvisionOnly => {
|
||||
run_provision_only(payload, log, sbx_dir)
|
||||
}
|
||||
SetupMode::Full => run_setup_full(payload, log, sbx_dir),
|
||||
}?;
|
||||
if writes_setup_marker {
|
||||
@@ -1090,6 +1093,15 @@ mod tests {
|
||||
assert_eq!(payload.mode, super::SetupMode::ProvisionOnly);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn payload_accepts_interactive_provision_mode() {
|
||||
let mut payload = payload_json();
|
||||
payload["mode"] = json!("interactive-provision");
|
||||
let payload: Payload = serde_json::from_value(payload).expect("payload");
|
||||
|
||||
assert_eq!(payload.mode, super::SetupMode::InteractiveProvision);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn payload_accepts_otel_settings() {
|
||||
let mut payload = payload_json();
|
||||
|
||||
@@ -216,13 +216,6 @@ pub fn require_logon_sandbox_creds(
|
||||
codex_home,
|
||||
proxy_enforced,
|
||||
},
|
||||
crate::setup::SetupRootOverrides {
|
||||
read_roots: Some(needed_read.clone()),
|
||||
read_roots_include_platform_defaults,
|
||||
write_roots: Some(needed_write.clone()),
|
||||
deny_read_paths: Some(deny_read_paths_override.to_vec()),
|
||||
deny_write_paths: Some(deny_write_paths_override.to_vec()),
|
||||
},
|
||||
&desired_offline_proxy_settings,
|
||||
)?;
|
||||
identity = select_identity(network_identity, codex_home)?;
|
||||
|
||||
@@ -693,6 +693,7 @@ struct ElevationPayload {
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
enum SetupMode {
|
||||
Full,
|
||||
InteractiveProvision,
|
||||
ProvisionOnly,
|
||||
}
|
||||
|
||||
@@ -1018,26 +1019,19 @@ fn run_setup_exe_payload(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn run_elevated_setup(
|
||||
request: SandboxSetupRequest<'_>,
|
||||
overrides: SetupRootOverrides,
|
||||
) -> Result<()> {
|
||||
run_elevated_setup_inner(
|
||||
request, overrides, /*offline_proxy_settings_override*/ None,
|
||||
)
|
||||
pub fn run_elevated_setup(request: SandboxSetupRequest<'_>) -> Result<()> {
|
||||
run_elevated_setup_inner(request, /*offline_proxy_settings_override*/ None)
|
||||
}
|
||||
|
||||
pub(crate) fn run_elevated_setup_with_proxy_settings(
|
||||
request: SandboxSetupRequest<'_>,
|
||||
overrides: SetupRootOverrides,
|
||||
offline_proxy_settings: &OfflineProxySettings,
|
||||
) -> Result<()> {
|
||||
run_elevated_setup_inner(request, overrides, Some(offline_proxy_settings))
|
||||
run_elevated_setup_inner(request, Some(offline_proxy_settings))
|
||||
}
|
||||
|
||||
fn run_elevated_setup_inner(
|
||||
request: SandboxSetupRequest<'_>,
|
||||
overrides: SetupRootOverrides,
|
||||
offline_proxy_settings_override: Option<&OfflineProxySettings>,
|
||||
) -> Result<()> {
|
||||
if !request.permissions.is_enforceable_by_windows_sandbox() {
|
||||
@@ -1051,28 +1045,7 @@ fn run_elevated_setup_inner(
|
||||
format!("failed to create sandbox dir {}: {err}", sbx_dir.display()),
|
||||
)
|
||||
})?;
|
||||
let (read_roots, write_roots) = build_payload_roots(&request, &overrides);
|
||||
let deny_read_paths = build_payload_deny_read_paths(overrides.deny_read_paths);
|
||||
let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths);
|
||||
let offline_proxy_settings =
|
||||
offline_proxy_settings_for_request(&request, offline_proxy_settings_override);
|
||||
let payload = ElevationPayload {
|
||||
version: SETUP_VERSION,
|
||||
offline_username: OFFLINE_USERNAME.to_string(),
|
||||
online_username: ONLINE_USERNAME.to_string(),
|
||||
codex_home: request.codex_home.to_path_buf(),
|
||||
command_cwd: request.command_cwd.to_path_buf(),
|
||||
read_roots,
|
||||
write_roots,
|
||||
deny_read_paths,
|
||||
deny_write_paths,
|
||||
proxy_ports: offline_proxy_settings.proxy_ports,
|
||||
allow_local_binding: offline_proxy_settings.allow_local_binding,
|
||||
real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()),
|
||||
otel: codex_otel::global_statsig_metrics_settings(),
|
||||
mode: SetupMode::Full,
|
||||
refresh_only: false,
|
||||
};
|
||||
let payload = elevated_provisioning_payload(&request, offline_proxy_settings_override);
|
||||
let needs_elevation = !is_elevated().map_err(|err| {
|
||||
failure(
|
||||
SetupErrorCode::OrchestratorElevationCheckFailed,
|
||||
@@ -1082,6 +1055,31 @@ fn run_elevated_setup_inner(
|
||||
run_setup_exe(&payload, needs_elevation, request.codex_home)
|
||||
}
|
||||
|
||||
fn elevated_provisioning_payload(
|
||||
request: &SandboxSetupRequest<'_>,
|
||||
offline_proxy_settings_override: Option<&OfflineProxySettings>,
|
||||
) -> ElevationPayload {
|
||||
let offline_proxy_settings =
|
||||
offline_proxy_settings_for_request(request, offline_proxy_settings_override);
|
||||
ElevationPayload {
|
||||
version: SETUP_VERSION,
|
||||
offline_username: OFFLINE_USERNAME.to_string(),
|
||||
online_username: ONLINE_USERNAME.to_string(),
|
||||
codex_home: request.codex_home.to_path_buf(),
|
||||
command_cwd: request.codex_home.to_path_buf(),
|
||||
read_roots: Vec::new(),
|
||||
write_roots: Vec::new(),
|
||||
deny_read_paths: Vec::new(),
|
||||
deny_write_paths: Vec::new(),
|
||||
proxy_ports: offline_proxy_settings.proxy_ports,
|
||||
allow_local_binding: offline_proxy_settings.allow_local_binding,
|
||||
real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()),
|
||||
otel: codex_otel::global_statsig_metrics_settings(),
|
||||
mode: SetupMode::InteractiveProvision,
|
||||
refresh_only: false,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn run_elevated_provisioning_setup(
|
||||
codex_home: &Path,
|
||||
real_user: &str,
|
||||
@@ -1539,6 +1537,43 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn elevated_setup_payload_contains_no_caller_acl_roots() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let command_cwd = tmp.path().join("caller-workspace");
|
||||
let codex_home = tmp.path().join("codex-home");
|
||||
fs::create_dir_all(&command_cwd).expect("create workspace");
|
||||
let permissions = permissions_for(
|
||||
&workspace_write_profile(
|
||||
workspace_roots_for(&command_cwd).as_slice(),
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
),
|
||||
workspace_roots_for(&command_cwd).as_slice(),
|
||||
);
|
||||
let request = super::SandboxSetupRequest {
|
||||
permissions: &permissions,
|
||||
command_cwd: &command_cwd,
|
||||
env_map: &HashMap::new(),
|
||||
codex_home: &codex_home,
|
||||
proxy_enforced: false,
|
||||
};
|
||||
|
||||
let payload = super::elevated_provisioning_payload(
|
||||
&request, /*offline_proxy_settings_override*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(payload.command_cwd, codex_home);
|
||||
assert_eq!(payload.read_roots, Vec::<PathBuf>::new());
|
||||
assert_eq!(payload.write_roots, Vec::<PathBuf>::new());
|
||||
assert_eq!(payload.deny_read_paths, Vec::<PathBuf>::new());
|
||||
assert_eq!(payload.deny_write_paths, Vec::<PathBuf>::new());
|
||||
assert!(matches!(
|
||||
payload.mode,
|
||||
super::SetupMode::InteractiveProvision
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn report_helper_failure_uses_setup_error_report_when_clear_succeeded() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user