mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use a private desktop for Windows sandbox instead of Winsta0\Default (#14400)
## Summary - launch Windows sandboxed children on a private desktop instead of `Winsta0\Default` - make private desktop the default while keeping `windows.sandbox_private_desktop=false` as the escape hatch - centralize process launch through the shared `create_process_as_user(...)` path - scope the private desktop ACL to the launching logon SID ## Why Today sandboxed Windows commands run on the visible shared desktop. That leaves an avoidable same-desktop attack surface for window interaction, spoofing, and related UI/input issues. This change moves sandboxed commands onto a dedicated per-launch desktop by default so the sandbox no longer shares `Winsta0\Default` with the user session. The implementation stays conservative on security with no silent fallback back to `Winsta0\Default` If private-desktop setup fails on a machine, users can still opt out explicitly with `windows.sandbox_private_desktop=false`. ## Validation - `cargo build -p codex-cli` - elevated-path `codex exec` desktop-name probe returned `CodexSandboxDesktop-*` - elevated-path `codex exec` smoke sweep for shell commands, nested `pwsh`, jobs, and hidden `notepad` launch - unelevated-path full private-desktop compatibility sweep via `codex exec` with `-c windows.sandbox=unelevated`
This commit is contained in:
committed by
GitHub
Unverified
parent
9c9867c9fa
commit
6b3d82daca
@@ -4214,6 +4214,10 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
network: None,
|
||||
sandbox_permissions,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: Some("test".to_string()),
|
||||
arg0: None,
|
||||
};
|
||||
@@ -4226,6 +4230,10 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: params.justification.clone(),
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
@@ -125,6 +125,10 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: Some("test".to_string()),
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
@@ -4082,6 +4082,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
windows_sandbox_private_desktop: true,
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
},
|
||||
enforce_residency: Constrained::allow_any(None),
|
||||
@@ -4219,6 +4220,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
windows_sandbox_private_desktop: true,
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
},
|
||||
enforce_residency: Constrained::allow_any(None),
|
||||
@@ -4354,6 +4356,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
windows_sandbox_private_desktop: true,
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
},
|
||||
enforce_residency: Constrained::allow_any(None),
|
||||
@@ -4475,6 +4478,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
windows_sandbox_private_desktop: true,
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
},
|
||||
enforce_residency: Constrained::allow_any(None),
|
||||
|
||||
@@ -58,6 +58,7 @@ use crate::unified_exec::DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS;
|
||||
use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS;
|
||||
use crate::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_mode;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_private_desktop;
|
||||
use codex_app_server_protocol::Tools;
|
||||
use codex_app_server_protocol::UserSavedConfig;
|
||||
use codex_protocol::config_types::AltScreenMode;
|
||||
@@ -189,6 +190,8 @@ pub struct Permissions {
|
||||
/// Effective Windows sandbox mode derived from `[windows].sandbox` or
|
||||
/// legacy feature keys.
|
||||
pub windows_sandbox_mode: Option<WindowsSandboxModeToml>,
|
||||
/// Whether the final Windows sandboxed child should run on a private desktop.
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
/// Optional macOS seatbelt extension profile used to extend default
|
||||
/// seatbelt permissions when running under seatbelt.
|
||||
pub macos_seatbelt_profile_extensions: Option<MacOsSeatbeltProfileExtensions>,
|
||||
@@ -1934,6 +1937,8 @@ impl Config {
|
||||
let configured_features = Features::from_config(&cfg, &config_profile, feature_overrides);
|
||||
let features = ManagedFeatures::from_configured(configured_features, feature_requirements)?;
|
||||
let windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg, &config_profile);
|
||||
let windows_sandbox_private_desktop =
|
||||
resolve_windows_sandbox_private_desktop(&cfg, &config_profile);
|
||||
let resolved_cwd = normalize_for_native_workdir({
|
||||
use std::env;
|
||||
|
||||
@@ -2394,6 +2399,7 @@ impl Config {
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
windows_sandbox_mode,
|
||||
windows_sandbox_private_desktop,
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
},
|
||||
enforce_residency: enforce_residency.value,
|
||||
|
||||
@@ -41,6 +41,9 @@ pub enum WindowsSandboxModeToml {
|
||||
#[schemars(deny_unknown_fields)]
|
||||
pub struct WindowsToml {
|
||||
pub sandbox: Option<WindowsSandboxModeToml>,
|
||||
/// Defaults to `true`. Set to `false` to launch the final sandboxed child
|
||||
/// process on `Winsta0\\Default` instead of a private desktop.
|
||||
pub sandbox_private_desktop: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
||||
@@ -81,6 +81,7 @@ pub struct ExecParams {
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
pub justification: Option<String>,
|
||||
pub arg0: Option<String>,
|
||||
}
|
||||
@@ -231,6 +232,7 @@ pub fn build_exec_request(
|
||||
network,
|
||||
sandbox_permissions,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
justification,
|
||||
arg0: _,
|
||||
} = params;
|
||||
@@ -271,6 +273,7 @@ pub fn build_exec_request(
|
||||
codex_linux_sandbox_exe: codex_linux_sandbox_exe.as_ref(),
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
})
|
||||
.map_err(CodexErr::from)?;
|
||||
Ok(exec_req)
|
||||
@@ -290,6 +293,7 @@ pub(crate) async fn execute_exec_request(
|
||||
expiration,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
sandbox_permissions,
|
||||
sandbox_policy: _sandbox_policy_from_env,
|
||||
file_system_sandbox_policy,
|
||||
@@ -307,6 +311,7 @@ pub(crate) async fn execute_exec_request(
|
||||
network: network.clone(),
|
||||
sandbox_permissions,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
justification,
|
||||
arg0,
|
||||
};
|
||||
@@ -409,6 +414,7 @@ async fn exec_windows_sandbox(
|
||||
network,
|
||||
expiration,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
..
|
||||
} = params;
|
||||
if let Some(network) = network.as_ref() {
|
||||
@@ -443,6 +449,7 @@ async fn exec_windows_sandbox(
|
||||
&cwd,
|
||||
env,
|
||||
timeout_ms,
|
||||
windows_sandbox_private_desktop,
|
||||
)
|
||||
} else {
|
||||
run_windows_sandbox_capture(
|
||||
@@ -453,6 +460,7 @@ async fn exec_windows_sandbox(
|
||||
&cwd,
|
||||
env,
|
||||
timeout_ms,
|
||||
windows_sandbox_private_desktop,
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -319,6 +319,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
};
|
||||
@@ -375,6 +376,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
@@ -69,6 +69,7 @@ pub struct ExecRequest {
|
||||
pub expiration: ExecExpiration,
|
||||
pub sandbox: SandboxType,
|
||||
pub windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
pub file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
@@ -96,6 +97,7 @@ pub(crate) struct SandboxTransformRequest<'a> {
|
||||
pub codex_linux_sandbox_exe: Option<&'a PathBuf>,
|
||||
pub use_legacy_landlock: bool,
|
||||
pub windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
}
|
||||
|
||||
pub enum SandboxPreference {
|
||||
@@ -593,6 +595,7 @@ impl SandboxManager {
|
||||
codex_linux_sandbox_exe,
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
} = request;
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
let macos_seatbelt_profile_extensions = None;
|
||||
@@ -705,6 +708,7 @@ impl SandboxManager {
|
||||
expiration: spec.expiration,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
sandbox_permissions: spec.sandbox_permissions,
|
||||
sandbox_policy: effective_policy,
|
||||
file_system_sandbox_policy: effective_file_system_policy,
|
||||
|
||||
@@ -169,6 +169,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network()
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
})
|
||||
.expect("transform");
|
||||
|
||||
@@ -502,6 +503,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() {
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
})
|
||||
.expect("transform");
|
||||
|
||||
@@ -574,6 +576,7 @@ fn transform_additional_permissions_preserves_denied_entries() {
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
})
|
||||
.expect("transform");
|
||||
|
||||
|
||||
@@ -167,6 +167,10 @@ pub(crate) async fn execute_user_shell_command(
|
||||
expiration: USER_SHELL_TIMEOUT_MS.into(),
|
||||
sandbox: SandboxType::None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
sandbox_policy: sandbox_policy.clone(),
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
|
||||
@@ -74,6 +74,10 @@ impl ShellHandler {
|
||||
network: turn_context.network.clone(),
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: params.justification.clone(),
|
||||
arg0: None,
|
||||
}
|
||||
@@ -124,6 +128,10 @@ impl ShellCommandHandler {
|
||||
network: turn_context.network.clone(),
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: params.justification.clone(),
|
||||
arg0: None,
|
||||
})
|
||||
|
||||
@@ -1067,6 +1067,10 @@ impl JsReplManager {
|
||||
codex_linux_sandbox_exe: turn.codex_linux_sandbox_exe.as_ref(),
|
||||
use_legacy_landlock: turn.features.use_legacy_landlock(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
})
|
||||
.map_err(|err| format!("failed to configure sandbox for js_repl: {err}"))?;
|
||||
|
||||
|
||||
@@ -197,6 +197,10 @@ impl ToolOrchestrator {
|
||||
codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(),
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level: turn_ctx.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_ctx
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
};
|
||||
|
||||
let (first_result, first_deferred_network_approval) = Self::run_attempt(
|
||||
@@ -319,6 +323,10 @@ impl ToolOrchestrator {
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level: turn_ctx.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_ctx
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
};
|
||||
|
||||
// Second attempt.
|
||||
|
||||
@@ -126,6 +126,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
expiration: _sandbox_expiration,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: _windows_sandbox_private_desktop,
|
||||
sandbox_permissions,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
@@ -924,6 +925,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
expiration: ExecExpiration::Cancellation(cancel_rx),
|
||||
sandbox: self.sandbox,
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: false,
|
||||
sandbox_permissions: self.sandbox_permissions,
|
||||
sandbox_policy: self.sandbox_policy.clone(),
|
||||
file_system_sandbox_policy: self.file_system_sandbox_policy.clone(),
|
||||
@@ -1080,6 +1082,7 @@ impl CoreShellCommandExecutor {
|
||||
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.as_ref(),
|
||||
use_legacy_landlock: self.use_legacy_landlock,
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: false,
|
||||
})?;
|
||||
if let Some(network) = exec_request.network.as_ref() {
|
||||
network.apply_to_env(&mut exec_request.env);
|
||||
|
||||
@@ -730,6 +730,7 @@ async fn prepare_escalated_exec_permissions_preserve_macos_seatbelt_extensions()
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
windows_sandbox_private_desktop: false,
|
||||
macos_seatbelt_profile_extensions: Some(MacOsSeatbeltProfileExtensions {
|
||||
macos_preferences: MacOsPreferencesPermission::ReadWrite,
|
||||
..Default::default()
|
||||
|
||||
@@ -333,6 +333,7 @@ pub(crate) struct SandboxAttempt<'a> {
|
||||
pub codex_linux_sandbox_exe: Option<&'a std::path::PathBuf>,
|
||||
pub use_legacy_landlock: bool,
|
||||
pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
}
|
||||
|
||||
impl<'a> SandboxAttempt<'a> {
|
||||
@@ -356,6 +357,7 @@ impl<'a> SandboxAttempt<'a> {
|
||||
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe,
|
||||
use_legacy_landlock: self.use_legacy_landlock,
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self.windows_sandbox_private_desktop,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,6 +75,19 @@ pub fn resolve_windows_sandbox_mode(
|
||||
.or_else(|| legacy_windows_sandbox_mode(cfg.features.as_ref()))
|
||||
}
|
||||
|
||||
pub fn resolve_windows_sandbox_private_desktop(cfg: &ConfigToml, profile: &ConfigProfile) -> bool {
|
||||
profile
|
||||
.windows
|
||||
.as_ref()
|
||||
.and_then(|windows| windows.sandbox_private_desktop)
|
||||
.or_else(|| {
|
||||
cfg.windows
|
||||
.as_ref()
|
||||
.and_then(|windows| windows.sandbox_private_desktop)
|
||||
})
|
||||
.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn legacy_windows_sandbox_keys_present(features: Option<&FeaturesToml>) -> bool {
|
||||
let Some(entries) = features.map(|features| &features.entries) else {
|
||||
return false;
|
||||
|
||||
@@ -77,12 +77,14 @@ fn resolve_windows_sandbox_mode_prefers_profile_windows() {
|
||||
let cfg = ConfigToml {
|
||||
windows: Some(WindowsToml {
|
||||
sandbox: Some(WindowsSandboxModeToml::Unelevated),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let profile = ConfigProfile {
|
||||
windows: Some(WindowsToml {
|
||||
sandbox: Some(WindowsSandboxModeToml::Elevated),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -130,3 +132,47 @@ fn resolve_windows_sandbox_mode_profile_legacy_false_blocks_top_level_legacy_tru
|
||||
|
||||
assert_eq!(resolve_windows_sandbox_mode(&cfg, &profile), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_windows_sandbox_private_desktop_prefers_profile_windows() {
|
||||
let cfg = ConfigToml {
|
||||
windows: Some(WindowsToml {
|
||||
sandbox: Some(WindowsSandboxModeToml::Unelevated),
|
||||
sandbox_private_desktop: Some(false),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let profile = ConfigProfile {
|
||||
windows: Some(WindowsToml {
|
||||
sandbox: Some(WindowsSandboxModeToml::Elevated),
|
||||
sandbox_private_desktop: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(resolve_windows_sandbox_private_desktop(&cfg, &profile));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_windows_sandbox_private_desktop_defaults_to_true() {
|
||||
assert!(resolve_windows_sandbox_private_desktop(
|
||||
&ConfigToml::default(),
|
||||
&ConfigProfile::default()
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_windows_sandbox_private_desktop_respects_explicit_cfg_value() {
|
||||
let cfg = ConfigToml {
|
||||
windows: Some(WindowsToml {
|
||||
sandbox_private_desktop: Some(false),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!resolve_windows_sandbox_private_desktop(
|
||||
&cfg,
|
||||
&ConfigProfile::default()
|
||||
));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user