mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
fix(sandboxing): propagate managed DNS policy
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
use codex_network_proxy::ManagedNetworkDomainPolicy;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -12,6 +14,36 @@ pub fn allow_network_for_proxy(enforce_managed_network: bool) -> bool {
|
||||
enforce_managed_network
|
||||
}
|
||||
|
||||
pub(crate) fn dns_domain_policy_for_proxy(
|
||||
enforce_managed_network: bool,
|
||||
managed_network: Option<&ManagedNetworkSandboxContext>,
|
||||
) -> Option<&ManagedNetworkDomainPolicy> {
|
||||
let managed_network = managed_network?;
|
||||
if !enforce_managed_network
|
||||
|| !managed_network.allow_local_binding
|
||||
|| managed_network.loopback_ports.is_empty()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
managed_network
|
||||
.domain_policy
|
||||
.as_ref()
|
||||
.filter(|policy| !policy.allowed_domains.is_empty())
|
||||
}
|
||||
|
||||
pub(crate) fn insert_dns_policy_args(args: &mut Vec<String>, policy: &ManagedNetworkDomainPolicy) {
|
||||
let Some(index) = args.iter().position(|arg| arg == "--") else {
|
||||
panic!("missing `--`");
|
||||
};
|
||||
args.splice(
|
||||
index..index,
|
||||
[
|
||||
"--dns-domain-policy".to_string(),
|
||||
serde_json::to_string(policy).unwrap_or_else(|err| panic!("{err}")),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
/// Converts the permission profile into the CLI invocation for
|
||||
/// `codex-linux-sandbox`.
|
||||
///
|
||||
|
||||
@@ -93,3 +93,34 @@ fn proxy_network_requires_managed_requirements() {
|
||||
true
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dns_domain_policy_requires_managed_local_binding_and_proxy_ports() {
|
||||
for flags in 0_u8..32 {
|
||||
let context = ManagedNetworkSandboxContext {
|
||||
loopback_ports: (flags & 1 != 0).then_some(43123).into_iter().collect(),
|
||||
allow_local_binding: flags & 2 != 0,
|
||||
domain_policy: (flags & 4 != 0).then(|| ManagedNetworkDomainPolicy {
|
||||
allowed_domains: (flags & 8 != 0)
|
||||
.then(|| "example.com".to_string())
|
||||
.into_iter()
|
||||
.collect(),
|
||||
denied_domains: Vec::new(),
|
||||
}),
|
||||
};
|
||||
assert_eq!(
|
||||
dns_domain_policy_for_proxy(flags & 16 != 0, Some(&context)).is_some(),
|
||||
flags == 31
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn linux_args_include_dns_domain_policy_immediately_before_separator() {
|
||||
let mut args = vec!["--".to_string()];
|
||||
insert_dns_policy_args(&mut args, &ManagedNetworkDomainPolicy::default());
|
||||
assert_eq!(
|
||||
args.join(" "),
|
||||
r#"--dns-domain-policy {"allowedDomains":[],"deniedDomains":[]} --"#
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ use crate::bwrap::is_wsl1;
|
||||
use crate::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use crate::landlock::allow_network_for_proxy;
|
||||
use crate::landlock::create_linux_sandbox_command_args_for_permission_profile;
|
||||
use crate::landlock::dns_domain_policy_for_proxy;
|
||||
use crate::landlock::insert_dns_policy_args;
|
||||
use crate::policy_transforms::effective_permission_profile;
|
||||
use crate::policy_transforms::should_require_platform_sandbox;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -335,7 +337,6 @@ impl SandboxManager {
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
} = request;
|
||||
#[cfg(target_os = "macos")]
|
||||
let managed_network = command.managed_network.as_ref();
|
||||
let additional_permissions = command.additional_permissions.take();
|
||||
let managed_mitm_ca_trust_bundle_path =
|
||||
@@ -402,6 +403,11 @@ impl SandboxManager {
|
||||
use_legacy_landlock,
|
||||
allow_proxy_network,
|
||||
);
|
||||
if let Some(domain_policy) =
|
||||
dns_domain_policy_for_proxy(enforce_managed_network, managed_network)
|
||||
{
|
||||
insert_dns_policy_args(&mut args, domain_policy);
|
||||
}
|
||||
let mut full_command = Vec::with_capacity(1 + args.len());
|
||||
full_command.push(os_string_to_command_component(exe.as_os_str().to_owned()));
|
||||
full_command.append(&mut args);
|
||||
|
||||
@@ -7,6 +7,10 @@ use super::SandboxType;
|
||||
use super::SandboxablePreference;
|
||||
use super::get_platform_sandbox;
|
||||
use super::with_managed_mitm_ca_readable_root;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_network_proxy::ManagedNetworkDomainPolicy;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
@@ -334,6 +338,38 @@ fn transform_linux_seccomp_request(
|
||||
.expect("transform")
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn transform_linux_seccomp_request_with_managed_network(
|
||||
managed_network: ManagedNetworkSandboxContext,
|
||||
) -> super::SandboxExecRequest {
|
||||
let manager = SandboxManager::new();
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("current dir");
|
||||
let cwd_uri = PathUri::from_abs_path(&cwd);
|
||||
let permissions = PermissionProfile::Disabled;
|
||||
manager
|
||||
.transform(SandboxTransformRequest {
|
||||
command: SandboxCommand {
|
||||
program: "true".into(),
|
||||
args: Vec::new(),
|
||||
cwd: cwd_uri.clone(),
|
||||
env: HashMap::new(),
|
||||
managed_network: Some(managed_network),
|
||||
additional_permissions: None,
|
||||
},
|
||||
permissions: &permissions,
|
||||
sandbox: SandboxType::LinuxSeccomp,
|
||||
enforce_managed_network: true,
|
||||
environment_id: None,
|
||||
network: None,
|
||||
sandbox_policy_cwd: &cwd_uri,
|
||||
codex_linux_sandbox_exe: Some(std::path::Path::new("/tmp/codex-linux-sandbox")),
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
})
|
||||
.expect("transform")
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn wsl1_rejects_linux_bubblewrap_path() {
|
||||
@@ -424,6 +460,64 @@ fn transform_linux_seccomp_uses_helper_alias_when_launcher_is_not_helper_path()
|
||||
assert_eq!(exec_request.arg0, Some("codex-linux-sandbox".to_string()));
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn transform_linux_seccomp_places_managed_dns_policy_before_command() {
|
||||
let domain_policy = ManagedNetworkDomainPolicy {
|
||||
allowed_domains: vec!["example.com".to_string()],
|
||||
denied_domains: vec!["blocked.example.com".to_string()],
|
||||
};
|
||||
let exec_request =
|
||||
transform_linux_seccomp_request_with_managed_network(ManagedNetworkSandboxContext {
|
||||
loopback_ports: vec![18080, 19090],
|
||||
allow_local_binding: true,
|
||||
domain_policy: Some(domain_policy.clone()),
|
||||
});
|
||||
|
||||
let separator_index = exec_request
|
||||
.command
|
||||
.iter()
|
||||
.position(|arg| arg == "--")
|
||||
.expect("sandbox argv separator");
|
||||
assert_eq!(
|
||||
exec_request
|
||||
.command
|
||||
.get(separator_index - 2)
|
||||
.map(String::as_str),
|
||||
Some("--dns-domain-policy")
|
||||
);
|
||||
let serialized_policy = exec_request
|
||||
.command
|
||||
.get(separator_index - 1)
|
||||
.expect("serialized DNS domain policy");
|
||||
assert_eq!(
|
||||
serde_json::from_str::<ManagedNetworkDomainPolicy>(serialized_policy)
|
||||
.expect("deserialize DNS domain policy"),
|
||||
domain_policy
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn transform_linux_seccomp_omits_managed_dns_policy_without_local_binding() {
|
||||
let exec_request =
|
||||
transform_linux_seccomp_request_with_managed_network(ManagedNetworkSandboxContext {
|
||||
loopback_ports: vec![18080, 19090],
|
||||
allow_local_binding: false,
|
||||
domain_policy: Some(ManagedNetworkDomainPolicy {
|
||||
allowed_domains: vec!["example.com".to_string()],
|
||||
denied_domains: Vec::new(),
|
||||
}),
|
||||
});
|
||||
|
||||
assert!(
|
||||
!exec_request
|
||||
.command
|
||||
.iter()
|
||||
.any(|arg| arg == "--dns-domain-policy")
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn transform_for_direct_spawn_windows_preserves_only_wrapper_setup_identity() {
|
||||
|
||||
Reference in New Issue
Block a user