mirror of
https://github.com/openai/codex.git
synced 2026-11-15 21:50:46 +00:00
Require elevated Windows policy for managed network
This commit is contained in:
@@ -54,6 +54,7 @@ use codex_app_server_protocol::TextPosition as AppTextPosition;
|
||||
use codex_app_server_protocol::TextRange as AppTextRange;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLoadError;
|
||||
use codex_config::ConstraintError;
|
||||
use codex_config::TextRange as CoreTextRange;
|
||||
use codex_core::ExecPolicyError;
|
||||
use codex_core::check_execpolicy_for_warnings;
|
||||
@@ -298,6 +299,12 @@ fn config_error_location(err: &std::io::Error) -> Option<(String, AppTextRange)>
|
||||
})
|
||||
}
|
||||
|
||||
fn is_unrecoverable_windows_network_config_error(err: &std::io::Error) -> bool {
|
||||
err.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<ConstraintError>())
|
||||
.is_some_and(ConstraintError::is_windows_network_configuration_error)
|
||||
}
|
||||
|
||||
fn exec_policy_warning_location(err: &ExecPolicyError) -> (Option<String>, Option<AppTextRange>) {
|
||||
match err {
|
||||
ExecPolicyError::ParsePolicy { path, source } => {
|
||||
@@ -521,7 +528,7 @@ pub async fn run_main_with_transport_options(
|
||||
{
|
||||
Ok(config) => (config, true),
|
||||
Err(err) => {
|
||||
if strict_config {
|
||||
if strict_config || is_unrecoverable_windows_network_config_error(&err) {
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
@@ -1364,6 +1371,7 @@ fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransp
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::LogFormat;
|
||||
use super::is_unrecoverable_windows_network_config_error;
|
||||
#[cfg(debug_assertions)]
|
||||
use super::loader_overrides_with_test_user_config_file;
|
||||
#[cfg(debug_assertions)]
|
||||
@@ -1372,6 +1380,23 @@ mod tests {
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn windows_network_config_errors_are_unrecoverable() {
|
||||
for error in [
|
||||
codex_config::ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox {
|
||||
requirement_source: codex_config::RequirementSource::Unknown,
|
||||
},
|
||||
codex_config::ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox,
|
||||
] {
|
||||
let error = std::io::Error::from(error);
|
||||
assert!(is_unrecoverable_windows_network_config_error(&error));
|
||||
}
|
||||
|
||||
assert!(!is_unrecoverable_windows_network_config_error(
|
||||
&std::io::Error::other("unrelated config error")
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn log_format_from_env_value_matches_json_values_case_insensitively() {
|
||||
assert_eq!(LogFormat::from_env_value(Some("json")), LogFormat::Json);
|
||||
|
||||
@@ -63,6 +63,15 @@ impl WindowsSandboxRequestProcessor {
|
||||
config.config_layer_stack.requirements(),
|
||||
params.mode,
|
||||
)?;
|
||||
let target_windows_sandbox_level = match setup_mode {
|
||||
CoreWindowsSandboxSetupMode::Elevated => WindowsSandboxLevel::Elevated,
|
||||
CoreWindowsSandboxSetupMode::Unelevated => WindowsSandboxLevel::RestrictedToken,
|
||||
};
|
||||
codex_core::config::validate_windows_sandbox_network_proxy_compatibility(
|
||||
target_windows_sandbox_level,
|
||||
config.permissions.network.is_some(),
|
||||
)
|
||||
.map_err(|err| invalid_request(err.to_string()))?;
|
||||
|
||||
self.outgoing
|
||||
.send_response(
|
||||
|
||||
@@ -1299,6 +1299,23 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(network) = &network {
|
||||
let elevated_is_only_allowed_windows_sandbox =
|
||||
windows.as_ref().is_some_and(|windows| {
|
||||
matches!(
|
||||
windows.value.allowed_sandbox_implementations.as_deref(),
|
||||
Some([WindowsSandboxModeToml::Elevated])
|
||||
)
|
||||
});
|
||||
if !elevated_is_only_allowed_windows_sandbox {
|
||||
return Err(
|
||||
ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox {
|
||||
requirement_source: network.source.clone(),
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let approval_policy = match allowed_approval_policies {
|
||||
Some(Sourced {
|
||||
value: policies,
|
||||
@@ -2795,6 +2812,63 @@ allowed_approvals_reviewers = ["user"]
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_network_requires_elevated_only_windows_sandbox() -> Result<()> {
|
||||
for windows_requirements in [
|
||||
"",
|
||||
r#"
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["unelevated"]
|
||||
"#,
|
||||
r#"
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated", "unelevated"]
|
||||
"#,
|
||||
] {
|
||||
let config: ConfigRequirementsToml = from_str(&format!(
|
||||
r#"
|
||||
[experimental_network]
|
||||
enabled = false
|
||||
|
||||
{windows_requirements}
|
||||
"#
|
||||
))?;
|
||||
|
||||
assert_eq!(
|
||||
ConfigRequirements::try_from(with_unknown_source(config)),
|
||||
Err(
|
||||
ConstraintError::ManagedNetworkRequiresElevatedWindowsSandbox {
|
||||
requirement_source: RequirementSource::Unknown,
|
||||
}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_network_accepts_elevated_only_windows_sandbox() -> Result<()> {
|
||||
let config: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
[experimental_network]
|
||||
enabled = true
|
||||
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated"]
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let requirements = ConfigRequirements::try_from(with_unknown_source(config))?;
|
||||
assert!(requirements.network.is_some());
|
||||
assert_eq!(
|
||||
requirements.windows_sandbox_mode.value(),
|
||||
Some(WindowsSandboxModeToml::Elevated)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_legacy_allowed_approvals_reviewer() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
@@ -3335,6 +3409,9 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
[experimental_network.unix_sockets]
|
||||
"/tmp/example.sock" = "allow"
|
||||
"/tmp/blocked.sock" = "deny"
|
||||
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated"]
|
||||
"#;
|
||||
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
@@ -3408,6 +3485,9 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
denied_domains = ["blocked.example.com"]
|
||||
allow_unix_sockets = ["/tmp/example.sock"]
|
||||
allow_local_binding = false
|
||||
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated"]
|
||||
"#;
|
||||
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
|
||||
@@ -33,6 +33,18 @@ pub enum ConstraintError {
|
||||
requirement_source: RequirementSource,
|
||||
reason: String,
|
||||
},
|
||||
|
||||
#[error(
|
||||
"managed network requirements (set by {requirement_source}) require `[windows] allowed_sandbox_implementations = [\"elevated\"]`"
|
||||
)]
|
||||
ManagedNetworkRequiresElevatedWindowsSandbox {
|
||||
requirement_source: RequirementSource,
|
||||
},
|
||||
|
||||
#[error(
|
||||
"the network proxy is incompatible with the unelevated Windows sandbox; set `windows.sandbox = \"elevated\"` or disable the network proxy"
|
||||
)]
|
||||
NetworkProxyIncompatibleWithUnelevatedWindowsSandbox,
|
||||
}
|
||||
|
||||
impl ConstraintError {
|
||||
@@ -41,6 +53,14 @@ impl ConstraintError {
|
||||
field_name: field_name.into(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_windows_network_configuration_error(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::ManagedNetworkRequiresElevatedWindowsSandbox { .. }
|
||||
| Self::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
pub type ConstraintResult<T> = Result<T, ConstraintError>;
|
||||
|
||||
@@ -99,6 +99,84 @@ use serde::Deserialize;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn windows_network_proxy_requires_elevated_sandbox() {
|
||||
let err = validate_windows_sandbox_network_proxy_compatibility_for_platform(
|
||||
/*is_windows*/ true,
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
/*network_proxy_configured*/ true,
|
||||
)
|
||||
.expect_err("unelevated Windows sandbox should reject configured network proxy");
|
||||
|
||||
assert!(matches!(
|
||||
err.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<ConstraintError>()),
|
||||
Some(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_network_proxy_compatibility_allows_valid_combinations() -> std::io::Result<()> {
|
||||
for (windows_sandbox_level, network_proxy_configured) in [
|
||||
(WindowsSandboxLevel::Disabled, false),
|
||||
(WindowsSandboxLevel::Disabled, true),
|
||||
(WindowsSandboxLevel::RestrictedToken, false),
|
||||
(WindowsSandboxLevel::Elevated, false),
|
||||
(WindowsSandboxLevel::Elevated, true),
|
||||
] {
|
||||
validate_windows_sandbox_network_proxy_compatibility_for_platform(
|
||||
/*is_windows*/ true,
|
||||
windows_sandbox_level,
|
||||
network_proxy_configured,
|
||||
)?;
|
||||
}
|
||||
validate_windows_sandbox_network_proxy_compatibility_for_platform(
|
||||
/*is_windows*/ false,
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
/*network_proxy_configured*/ true,
|
||||
)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[tokio::test]
|
||||
async fn hand_edited_network_proxy_and_unelevated_sandbox_is_rejected() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?;
|
||||
|
||||
let err = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
|
||||
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
|
||||
network_access: true,
|
||||
..Default::default()
|
||||
}),
|
||||
features: Some(toml::from_str("network_proxy = true").expect("valid features")),
|
||||
windows: Some(WindowsToml {
|
||||
sandbox: Some(WindowsSandboxModeToml::Unelevated),
|
||||
sandbox_private_desktop: None,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await
|
||||
.expect_err("hand-edited incompatible network and sandbox settings should fail");
|
||||
|
||||
assert!(matches!(
|
||||
err.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<ConstraintError>()),
|
||||
Some(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox)
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::TempDirExt;
|
||||
@@ -1670,6 +1748,9 @@ async fn experimental_network_requirements_enable_proxy_without_feature() -> std
|
||||
r#"
|
||||
[experimental_network]
|
||||
enabled = true
|
||||
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated"]
|
||||
"#,
|
||||
),
|
||||
)
|
||||
|
||||
@@ -597,6 +597,31 @@ fn build_network_proxy_spec(
|
||||
})
|
||||
}
|
||||
|
||||
pub fn validate_windows_sandbox_network_proxy_compatibility(
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
network_proxy_configured: bool,
|
||||
) -> std::io::Result<()> {
|
||||
validate_windows_sandbox_network_proxy_compatibility_for_platform(
|
||||
cfg!(target_os = "windows"),
|
||||
windows_sandbox_level,
|
||||
network_proxy_configured,
|
||||
)
|
||||
}
|
||||
|
||||
fn validate_windows_sandbox_network_proxy_compatibility_for_platform(
|
||||
is_windows: bool,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
network_proxy_configured: bool,
|
||||
) -> std::io::Result<()> {
|
||||
if is_windows
|
||||
&& network_proxy_configured
|
||||
&& windows_sandbox_level == WindowsSandboxLevel::RestrictedToken
|
||||
{
|
||||
return Err(ConstraintError::NetworkProxyIncompatibleWithUnelevatedWindowsSandbox.into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Configured thread persistence backend.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub enum ThreadStoreConfig {
|
||||
@@ -3738,6 +3763,10 @@ impl Config {
|
||||
network_requirements,
|
||||
&network_permission_profile,
|
||||
)?;
|
||||
validate_windows_sandbox_network_proxy_compatibility(
|
||||
windows_sandbox_level,
|
||||
network.is_some(),
|
||||
)?;
|
||||
let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime(
|
||||
&codex_home,
|
||||
zsh_path.as_ref(),
|
||||
@@ -4132,11 +4161,16 @@ impl Config {
|
||||
NetworkProxyConfig::default()
|
||||
};
|
||||
|
||||
build_network_proxy_spec(
|
||||
let network = build_network_proxy_spec(
|
||||
configured_network_proxy_config,
|
||||
self.config_layer_stack.requirements().network.clone(),
|
||||
permission_profile,
|
||||
)
|
||||
)?;
|
||||
validate_windows_sandbox_network_proxy_compatibility(
|
||||
WindowsSandboxLevel::from_config(self),
|
||||
network.is_some(),
|
||||
)?;
|
||||
Ok(network)
|
||||
}
|
||||
|
||||
pub fn bundled_skills_enabled(&self) -> bool {
|
||||
|
||||
@@ -17,6 +17,7 @@ use tokio::io::BufReader;
|
||||
use tokio::process::Child;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::config::validate_windows_sandbox_network_proxy_compatibility;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
@@ -411,10 +412,13 @@ pub fn build_exec_request(
|
||||
)
|
||||
})
|
||||
.map_err(CodexErr::from)?;
|
||||
let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend(
|
||||
validate_windows_sandbox_network_proxy_compatibility(
|
||||
exec_req.windows_sandbox_level,
|
||||
exec_req.network.is_some(),
|
||||
);
|
||||
)
|
||||
.map_err(CodexErr::Io)?;
|
||||
let use_windows_elevated_backend =
|
||||
windows_sandbox_uses_elevated_backend(exec_req.windows_sandbox_level);
|
||||
exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend {
|
||||
resolve_windows_elevated_filesystem_overrides(
|
||||
exec_req.sandbox,
|
||||
@@ -658,7 +662,9 @@ async fn exec_windows_sandbox(
|
||||
let command_path = command.first().cloned();
|
||||
let sandbox_level = windows_sandbox_level;
|
||||
let proxy_enforced = network.is_some();
|
||||
let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced);
|
||||
validate_windows_sandbox_network_proxy_compatibility(sandbox_level, proxy_enforced)
|
||||
.map_err(CodexErr::Io)?;
|
||||
let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level);
|
||||
let additional_deny_write_paths = windows_sandbox_filesystem_overrides
|
||||
.map(|overrides| overrides.additional_deny_write_paths.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
@@ -397,18 +397,12 @@ fn windows_restricted_token_supports_read_only_profiles() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_proxy_enforcement_uses_elevated_backend() {
|
||||
fn windows_backend_selection_uses_configured_sandbox_level() {
|
||||
assert!(!windows_sandbox_uses_elevated_backend(
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
/*proxy_enforced*/ false,
|
||||
));
|
||||
assert!(windows_sandbox_uses_elevated_backend(
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
/*proxy_enforced*/ true,
|
||||
));
|
||||
assert!(windows_sandbox_uses_elevated_backend(
|
||||
WindowsSandboxLevel::Elevated,
|
||||
/*proxy_enforced*/ false,
|
||||
));
|
||||
}
|
||||
|
||||
|
||||
@@ -223,6 +223,9 @@ pub fn managed_network_requirements_loader() -> CloudConfigBundleLoader {
|
||||
[experimental_network]
|
||||
enabled = true
|
||||
allow_local_binding = true
|
||||
|
||||
[windows]
|
||||
allowed_sandbox_implementations = ["elevated"]
|
||||
"#,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -532,8 +532,7 @@ fn wrap_windows_sandbox_exec_request_for_direct_spawn(
|
||||
|
||||
let inner_command = std::mem::take(&mut request.command);
|
||||
let proxy_enforced = request.network.is_some();
|
||||
let use_elevated =
|
||||
windows_sandbox_uses_elevated_backend(request.windows_sandbox_level, proxy_enforced);
|
||||
let use_elevated = windows_sandbox_uses_elevated_backend(request.windows_sandbox_level);
|
||||
let overrides = if use_elevated {
|
||||
resolve_windows_elevated_filesystem_overrides(
|
||||
request.sandbox,
|
||||
|
||||
@@ -29,14 +29,8 @@ pub struct WindowsSandboxFilesystemOverrides {
|
||||
pub additional_deny_write_paths: Vec<AbsolutePathBuf>,
|
||||
}
|
||||
|
||||
pub fn windows_sandbox_uses_elevated_backend(
|
||||
sandbox_level: WindowsSandboxLevel,
|
||||
proxy_enforced: bool,
|
||||
) -> bool {
|
||||
// Windows firewall enforcement is tied to the logon-user sandbox identities, so
|
||||
// proxy-enforced sessions must use that backend even when the configured mode is
|
||||
// the default restricted-token sandbox.
|
||||
proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated)
|
||||
pub fn windows_sandbox_uses_elevated_backend(sandbox_level: WindowsSandboxLevel) -> bool {
|
||||
matches!(sandbox_level, WindowsSandboxLevel::Elevated)
|
||||
}
|
||||
|
||||
pub fn permission_profile_supports_windows_restricted_token_sandbox(
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
mod backends;
|
||||
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -47,8 +48,12 @@ pub async fn spawn_windows_sandbox_session_for_level(
|
||||
request: WindowsSandboxSessionRequest<'_>,
|
||||
) -> Result<SpawnedProcess> {
|
||||
if request.proxy_enforced
|
||||
|| matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated)
|
||||
&& !matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated)
|
||||
{
|
||||
bail!("network proxy enforcement requires the elevated Windows sandbox backend");
|
||||
}
|
||||
|
||||
if matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated) {
|
||||
backends::elevated::spawn_windows_sandbox_session_elevated_for_permission_profile(
|
||||
request.permission_profile,
|
||||
request.workspace_roots,
|
||||
|
||||
Reference in New Issue
Block a user