diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a00873ee48d5..43c0cd59daa8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4059,6 +4059,7 @@ dependencies = [ "assert_cmd", "base64 0.22.1", "codex-arg0", + "codex-config", "codex-core", "codex-exec-server", "codex-features", diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index a1692b2cb169..3bd19f556882 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -114,7 +114,8 @@ pub struct ConfigToml { /// Sandbox configuration to apply if `sandbox` is `WorkspaceWrite`. pub sandbox_workspace_write: Option, - /// Default named permissions profile to apply from the `[permissions]` + /// Default permissions profile to apply. Names starting with `:` refer to + /// built-in profiles; other names are resolved from the `[permissions]` /// table. pub default_permissions: Option, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index bdccafcb53c0..0758d14ba94e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2494,7 +2494,7 @@ "type": "string" }, "default_permissions": { - "description": "Default named permissions profile to apply from the `[permissions]` table.", + "description": "Default permissions profile to apply. Names starting with `:` refer to built-in profiles; other names are resolved from the `[permissions]` table.", "type": "string" }, "developer_instructions": { diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 21a86dcadd94..e74ebaf9bf76 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -49,6 +49,8 @@ use codex_config::types::SkillsConfig; use codex_config::types::ToolSuggestDiscoverableType; use codex_config::types::Tui; use codex_config::types::TuiNotificationSettings; +use codex_config::types::WindowsSandboxModeToml; +use codex_config::types::WindowsToml; use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_features::FeaturesToml; @@ -661,8 +663,55 @@ allow_upstream_proxy = false } #[tokio::test] -async fn permissions_profiles_network_populates_runtime_network_proxy_spec() -> std::io::Result<()> -{ +async fn permissions_profiles_network_enabled_allows_runtime_network_without_proxy() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("workspace".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "workspace".to_string(), + PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":minimal".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), + network: Some(NetworkToml { + enabled: Some(true), + ..Default::default() + }), + }, + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); + assert!( + config.permissions.network.is_none(), + "bare profile network.enabled should not start the managed network proxy" + ); + Ok(()) +} + +#[tokio::test] +async fn permissions_profiles_proxy_policy_starts_managed_network_proxy() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; @@ -699,14 +748,20 @@ async fn permissions_profiles_network_populates_runtime_network_proxy_spec() -> codex_home.abs(), ) .await?; + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); let network = config .permissions .network .as_ref() - .expect("enabled profile network should produce a NetworkProxySpec"); - + .expect("profile proxy policy should start the managed network proxy"); assert_eq!(network.proxy_host_and_port(), "127.0.0.1:43128"); - assert!(!network.socks_enabled()); + assert!( + !network.socks_enabled(), + "profile proxy policy should preserve SOCKS config" + ); Ok(()) } @@ -1014,7 +1069,8 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection( } #[tokio::test] -async fn permission_profile_override_preserves_configured_network_proxy() -> std::io::Result<()> { +async fn permission_profile_override_preserves_configured_network_policy_without_starting_proxy() +-> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; let permission_profile = PermissionProfile::Disabled; @@ -1059,14 +1115,10 @@ async fn permission_profile_override_preserves_configured_network_proxy() -> std codex_home.abs(), ) .await?; - let network = config - .permissions - .network - .as_ref() - .expect("network-enabled override should preserve configured proxy"); - - assert_eq!(network.proxy_host_and_port(), "127.0.0.1:43128"); - assert!(!network.socks_enabled()); + assert!( + config.permissions.network.is_none(), + "profile network.enabled should not start the managed network proxy" + ); assert_eq!(config.permissions.permission_profile(), permission_profile); Ok(()) } @@ -1189,6 +1241,306 @@ async fn permissions_profiles_require_default_permissions() -> std::io::Result<( Ok(()) } +#[tokio::test] +async fn default_permissions_can_select_builtin_profile_without_permissions_table() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some(":workspace".to_string()), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected :workspace to allow writing the project root, policy: {policy:?}" + ); + assert!( + !policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()), + "expected :workspace to protect project metadata, policy: {policy:?}" + ); + Ok(()) +} + +#[tokio::test] +async fn empty_config_defaults_to_builtin_profile_for_trusted_project() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let project_key = cwd.path().to_string_lossy().to_string(); + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + projects: Some(HashMap::from([( + project_key, + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + if cfg!(target_os = "windows") { + assert!( + !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected trusted project fallback to stay read-only without Windows sandbox support, policy: {policy:?}" + ); + } else { + assert!( + policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected trusted project fallback to use :workspace, policy: {policy:?}" + ); + assert!( + !policy.can_write_path_with_cwd(&cwd.path().join(".codex"), cwd.path()), + "expected :workspace metadata carveouts, policy: {policy:?}" + ); + } + Ok(()) +} + +#[tokio::test] +async fn implicit_builtin_workspace_profile_preserves_sandbox_workspace_write_settings() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let extra_root = TempDir::new()?; + let extra_root = extra_root.path().abs(); + let project_key = cwd.path().to_string_lossy().to_string(); + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + projects: Some(HashMap::from([( + project_key, + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + sandbox_workspace_write: Some(SandboxWorkspaceWrite { + writable_roots: vec![extra_root.clone()], + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: false, + }), + windows: Some(WindowsToml { + sandbox: Some(WindowsSandboxModeToml::Elevated), + sandbox_private_desktop: None, + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_write_path_with_cwd(extra_root.as_path(), cwd.path()), + "expected implicit :workspace to preserve sandbox_workspace_write.writable_roots, policy: {policy:?}" + ); + assert_eq!( + config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Enabled + ); + match config.legacy_sandbox_policy() { + SandboxPolicy::WorkspaceWrite { + writable_roots, + network_access, + exclude_tmpdir_env_var, + exclude_slash_tmp, + } => { + assert!(writable_roots.contains(&extra_root)); + assert!(network_access); + assert!(exclude_tmpdir_env_var); + assert!(!exclude_slash_tmp); + } + sandbox_policy => panic!("expected workspace-write projection, got {sandbox_policy:?}"), + } + Ok(()) +} + +#[tokio::test] +async fn implicit_builtin_workspace_profile_preserves_add_dir_metadata_carveouts() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let extra_root = TempDir::new()?; + for subpath in [".git", ".agents", ".codex"] { + std::fs::create_dir_all(extra_root.path().join(subpath))?; + } + let project_key = cwd.path().to_string_lossy().to_string(); + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + projects: Some(HashMap::from([( + project_key, + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + windows: Some(WindowsToml { + sandbox: Some(WindowsSandboxModeToml::Elevated), + sandbox_private_desktop: None, + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + additional_writable_roots: vec![extra_root.path().to_path_buf()], + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + let extra_root = extra_root.path().abs(); + assert!( + policy.can_write_path_with_cwd(extra_root.as_path(), cwd.path()), + "expected implicit :workspace to preserve additional writable roots, policy: {policy:?}" + ); + for subpath in [".git", ".agents", ".codex"] { + assert!( + !policy.can_write_path_with_cwd(&extra_root.join(subpath), cwd.path()), + "expected implicit :workspace to preserve legacy metadata carveout for {subpath}, \ + policy: {policy:?}" + ); + } + Ok(()) +} + +#[tokio::test] +async fn empty_config_defaults_to_builtin_read_only_without_trust_decision() -> std::io::Result<()> +{ + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + let policy = config.permissions.file_system_sandbox_policy(); + assert!( + policy.can_read_path_with_cwd(cwd.path(), cwd.path()), + "expected :read-only to allow reads, policy: {policy:?}" + ); + assert!( + !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + "expected :read-only to deny writes, policy: {policy:?}" + ); + Ok(()) +} + +#[tokio::test] +async fn default_permissions_can_select_builtin_no_sandbox_profile() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some(":danger-no-sandbox".to_string()), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.permission_profile(), + PermissionProfile::Disabled + ); + Ok(()) +} + +#[tokio::test] +async fn user_defined_permission_profile_names_cannot_use_builtin_prefix() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let err = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some(":custom".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + ":custom".to_string(), + PermissionProfileToml::default(), + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await + .expect_err("reserved profile name should be rejected"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + "permissions profile `:custom` uses a reserved built-in profile prefix" + ); + Ok(()) +} + +#[tokio::test] +async fn unknown_builtin_permission_profile_name_is_rejected() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let err = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some(":unknown".to_string()), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.abs(), + ) + .await + .expect_err("unknown built-in profile name should be rejected"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + "default_permissions refers to unknown built-in profile `:unknown`" + ); + Ok(()) +} + #[tokio::test] async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() -> std::io::Result<()> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index f4a73dd25f82..4e14ffd2fb64 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -99,9 +99,12 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use crate::config::permissions::compile_permission_profile; +use crate::config::permissions::builtin_permission_profile; +use crate::config::permissions::compile_permission_profile_selection; +use crate::config::permissions::default_builtin_permission_profile_name; use crate::config::permissions::get_readable_roots_required_for_codex_runtime; -use crate::config::permissions::network_proxy_config_from_profile_network; +use crate::config::permissions::network_proxy_config_for_profile_selection; +use crate::config::permissions::validate_user_permission_profile_names; use codex_network_proxy::NetworkProxyConfig; use toml::Value as TomlValue; use toml_edit::DocumentMut; @@ -318,6 +321,18 @@ impl Permissions { } } +// A profile override only inherits the selected profile's proxy/allowlist config +// when Codex is still responsible for the network policy. `Disabled` means no +// outer sandbox, so starting the managed proxy would narrow the override. +fn profile_allows_configured_network_proxy(permission_profile: &PermissionProfile) -> bool { + match permission_profile { + PermissionProfile::Managed { network, .. } | PermissionProfile::External { network } => { + network.is_enabled() + } + PermissionProfile::Disabled => false, + } +} + /// Configured thread persistence backend. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ThreadStoreConfig { @@ -1861,6 +1876,7 @@ impl Config { .permissions .as_ref() .is_some_and(|profiles| !profiles.is_empty()); + validate_user_permission_profile_names(cfg.permissions.as_ref())?; if has_permission_profiles && !matches!( permission_config_syntax, @@ -1891,8 +1907,8 @@ impl Config { let profiles_are_active = matches!( permission_config_syntax, Some(PermissionConfigSyntax::Profiles) - ) || (permission_config_syntax.is_none() - && has_permission_profiles); + ) || permission_config_syntax.is_none(); + let using_implicit_builtin_profile = permission_config_syntax.is_none(); let ( configured_network_proxy_config, permission_profile, @@ -1901,25 +1917,22 @@ impl Config { let (mut file_system_sandbox_policy, network_sandbox_policy) = permission_profile.to_runtime_permissions(); let configured_network_proxy_config = - if network_sandbox_policy.is_enabled() && profiles_are_active { - let permissions = cfg.permissions.as_ref().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "default_permissions requires a `[permissions]` table", - ) - })?; - let default_permissions = cfg.default_permissions.as_deref().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "default_permissions requires a named permissions profile", - ) - })?; - let profile = resolve_permission_profile(permissions, default_permissions)?; - + if profile_allows_configured_network_proxy(&permission_profile) + && profiles_are_active + { // PermissionProfile carries the active network sandbox bit, not the configured // proxy/allowlist policy. Keep that config so active profiles can round-trip // without broadening network behavior. - network_proxy_config_from_profile_network(profile.network.as_ref()) + let default_permissions = cfg.default_permissions.as_deref().unwrap_or_else(|| { + default_builtin_permission_profile_name( + &active_project, + windows_sandbox_level, + ) + }); + network_proxy_config_for_profile_selection( + cfg.permissions.as_ref(), + default_permissions, + )? } else { NetworkProxyConfig::default() }; @@ -1947,32 +1960,31 @@ impl Config { file_system_sandbox_policy, ) } else if profiles_are_active { - let permissions = cfg.permissions.as_ref().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "default_permissions requires a `[permissions]` table", - ) - })?; - let default_permissions = cfg.default_permissions.as_deref().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "default_permissions requires a named permissions profile", - ) - })?; - let profile = resolve_permission_profile(permissions, default_permissions)?; - let configured_network_proxy_config = - network_proxy_config_from_profile_network(profile.network.as_ref()); + let default_permissions = cfg.default_permissions.as_deref().unwrap_or_else(|| { + default_builtin_permission_profile_name(&active_project, windows_sandbox_level) + }); + let configured_network_proxy_config = network_proxy_config_for_profile_selection( + cfg.permissions.as_ref(), + default_permissions, + )?; let (mut file_system_sandbox_policy, network_sandbox_policy) = - compile_permission_profile( - permissions, + compile_permission_profile_selection( + cfg.permissions.as_ref(), default_permissions, + cfg.sandbox_workspace_write.as_ref(), resolved_cwd.as_path(), &mut startup_warnings, )?; - let mut permission_profile = PermissionProfile::from_runtime_permissions( - &file_system_sandbox_policy, - network_sandbox_policy, - ); + let mut permission_profile = if let Some(permission_profile) = + builtin_permission_profile(default_permissions, cfg.sandbox_workspace_write.as_ref()) + { + permission_profile + } else { + PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + network_sandbox_policy, + ) + }; let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( &permission_profile, &file_system_sandbox_policy, @@ -1980,11 +1992,17 @@ impl Config { resolved_cwd.as_path(), ); if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { - file_system_sandbox_policy = file_system_sandbox_policy - .with_additional_writable_roots( + file_system_sandbox_policy = if using_implicit_builtin_profile { + file_system_sandbox_policy + .with_additional_legacy_workspace_writable_roots( + &additional_writable_roots, + ) + } else { + file_system_sandbox_policy.with_additional_writable_roots( resolved_cwd.as_path(), &additional_writable_roots, - ); + ) + }; permission_profile = PermissionProfile::from_runtime_permissions( &file_system_sandbox_policy, network_sandbox_policy, diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 6d938e918544..b51a8973a31c 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -9,9 +9,12 @@ use codex_config::permissions_toml::FilesystemPermissionsToml; use codex_config::permissions_toml::NetworkToml; use codex_config::permissions_toml::PermissionProfileToml; use codex_config::permissions_toml::PermissionsToml; +use codex_config::types::SandboxWorkspaceWrite; use codex_network_proxy::NetworkProxyConfig; #[cfg(test)] use codex_network_proxy::NetworkUnixSocketPermission as ProxyNetworkUnixSocketPermission; +use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -20,13 +23,120 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use super::ProjectConfig; + +pub(crate) const BUILT_IN_READ_ONLY_PROFILE: &str = ":read-only"; +pub(crate) const BUILT_IN_WORKSPACE_PROFILE: &str = ":workspace"; +pub(crate) const BUILT_IN_DANGER_NO_SANDBOX_PROFILE: &str = ":danger-no-sandbox"; + +pub(crate) fn default_builtin_permission_profile_name( + active_project: &ProjectConfig, + windows_sandbox_level: WindowsSandboxLevel, +) -> &'static str { + if (active_project.is_trusted() || active_project.is_untrusted()) + && !(cfg!(target_os = "windows") && windows_sandbox_level == WindowsSandboxLevel::Disabled) + { + BUILT_IN_WORKSPACE_PROFILE + } else { + BUILT_IN_READ_ONLY_PROFILE + } +} + +pub(crate) fn is_builtin_permission_profile_name(profile_name: &str) -> bool { + matches!( + profile_name, + BUILT_IN_READ_ONLY_PROFILE + | BUILT_IN_WORKSPACE_PROFILE + | BUILT_IN_DANGER_NO_SANDBOX_PROFILE + ) +} + +pub(crate) fn builtin_permission_profile( + profile_name: &str, + workspace_write: Option<&SandboxWorkspaceWrite>, +) -> Option { + match profile_name { + BUILT_IN_READ_ONLY_PROFILE => Some(PermissionProfile::read_only()), + BUILT_IN_WORKSPACE_PROFILE => Some(match workspace_write { + Some(SandboxWorkspaceWrite { + writable_roots, + network_access, + exclude_tmpdir_env_var, + exclude_slash_tmp, + }) => PermissionProfile::workspace_write_with( + writable_roots, + if *network_access { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ), + None => PermissionProfile::workspace_write(), + }), + BUILT_IN_DANGER_NO_SANDBOX_PROFILE => Some(PermissionProfile::Disabled), + _ => None, + } +} + +pub(crate) fn validate_user_permission_profile_names( + permissions: Option<&PermissionsToml>, +) -> io::Result<()> { + let Some(permissions) = permissions else { + return Ok(()); + }; + + for profile_name in permissions.entries.keys() { + if profile_name.starts_with(':') { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "permissions profile `{profile_name}` uses a reserved built-in profile prefix" + ), + )); + } + } + + Ok(()) +} + pub(crate) fn network_proxy_config_from_profile_network( network: Option<&NetworkToml>, ) -> NetworkProxyConfig { - network.map_or_else( + let mut config = network.map_or_else( NetworkProxyConfig::default, NetworkToml::to_network_proxy_config, - ) + ); + // Profile `network.enabled` controls sandbox network access. Do not start a + // managed proxy for that bit alone, but keep the proxy enabled when the + // profile also supplied policy that only the proxy can enforce. + config.network.enabled = network.is_some_and(profile_network_requires_proxy); + config +} + +fn profile_network_requires_proxy(network: &NetworkToml) -> bool { + if network.enabled != Some(true) { + return false; + } + + network.proxy_url.is_some() + || network.enable_socks5 == Some(true) + || network.socks_url.is_some() + || network.enable_socks5_udp == Some(true) + || network.allow_upstream_proxy == Some(true) + || network.dangerously_allow_non_loopback_proxy == Some(true) + || network.dangerously_allow_all_unix_sockets == Some(true) + || network.mode.is_some() + || network + .domains + .as_ref() + .is_some_and(|domains| !domains.is_empty()) + || network + .unix_sockets + .as_ref() + .is_some_and(|unix_sockets| !unix_sockets.is_empty()) + || network.allow_local_binding == Some(true) } pub(crate) fn resolve_permission_profile<'a>( @@ -41,6 +151,27 @@ pub(crate) fn resolve_permission_profile<'a>( }) } +pub(crate) fn network_proxy_config_for_profile_selection( + permissions: Option<&PermissionsToml>, + profile_name: &str, +) -> io::Result { + if is_builtin_permission_profile_name(profile_name) { + return Ok(NetworkProxyConfig::default()); + } + reject_unknown_builtin_permission_profile(profile_name)?; + + let permissions = permissions.ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "default_permissions requires a `[permissions]` table", + ) + })?; + let profile = resolve_permission_profile(permissions, profile_name)?; + Ok(network_proxy_config_from_profile_network( + profile.network.as_ref(), + )) +} + pub(crate) fn compile_permission_profile( permissions: &PermissionsToml, profile_name: &str, @@ -103,6 +234,38 @@ pub(crate) fn compile_permission_profile( Ok((file_system_sandbox_policy, network_sandbox_policy)) } +pub(crate) fn compile_permission_profile_selection( + permissions: Option<&PermissionsToml>, + profile_name: &str, + workspace_write: Option<&SandboxWorkspaceWrite>, + policy_cwd: &Path, + startup_warnings: &mut Vec, +) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> { + if let Some(permission_profile) = builtin_permission_profile(profile_name, workspace_write) { + return Ok(permission_profile.to_runtime_permissions()); + } + reject_unknown_builtin_permission_profile(profile_name)?; + + let permissions = permissions.ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "default_permissions requires a `[permissions]` table", + ) + })?; + compile_permission_profile(permissions, profile_name, policy_cwd, startup_warnings) +} + +fn reject_unknown_builtin_permission_profile(profile_name: &str) -> io::Result<()> { + if profile_name.starts_with(':') { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("default_permissions refers to unknown built-in profile `{profile_name}`"), + )); + } + + Ok(()) +} + /// Returns a list of paths that must be readable by shell tools in order /// for Codex to function. These should always be added to the /// `FileSystemSandboxPolicy` for a thread. diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index e021db3d2dd3..7dd2bcdefe08 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -236,6 +236,45 @@ fn network_toml_overlays_unix_socket_permissions_by_path() { ); } +#[test] +fn profile_network_proxy_config_keeps_proxy_disabled_for_bare_network_access() { + let config = network_proxy_config_from_profile_network(Some(&NetworkToml { + enabled: Some(true), + ..Default::default() + })); + + assert!(!config.network.enabled); +} + +#[test] +fn profile_network_proxy_config_enables_proxy_for_proxy_policy() { + let config = network_proxy_config_from_profile_network(Some(&NetworkToml { + enabled: Some(true), + proxy_url: Some("http://127.0.0.1:43128".to_string()), + enable_socks5: Some(false), + domains: Some(NetworkDomainPermissionsToml { + entries: BTreeMap::from([( + "openai.com".to_string(), + NetworkDomainPermissionToml::Allow, + )]), + }), + ..Default::default() + })); + + assert!(config.network.enabled); + assert_eq!(config.network.proxy_url, "http://127.0.0.1:43128"); + assert!(!config.network.enable_socks5); + assert_eq!( + config.network.domains, + Some(codex_network_proxy::NetworkDomainPermissions { + entries: vec![codex_network_proxy::NetworkDomainPermissionEntry { + pattern: "openai.com".to_string(), + permission: codex_network_proxy::NetworkDomainPermission::Allow, + }], + }) + ); +} + #[test] fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths() { let filesystem = FilesystemPermissionsToml { diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index e2765e8be6f2..f710aa36cc9b 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -15,6 +15,7 @@ anyhow = { workspace = true } assert_cmd = { workspace = true } base64 = { workspace = true } codex-arg0 = { workspace = true } +codex-config = { workspace = true } codex-core = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index a11d5ee6a478..c89e6a5188bc 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -8,6 +8,9 @@ use ctor::ctor; use std::sync::OnceLock; use tempfile::TempDir; +use codex_config::CloudRequirementsLoader; +use codex_config::ConfigRequirementsToml; +use codex_config::NetworkRequirementsToml; use codex_core::CodexThread; use codex_core::config::Config; use codex_core::config::ConfigBuilder; @@ -164,14 +167,41 @@ pub fn fetch_dotslash_file( /// temporary directory. Using a per-test directory keeps tests hermetic and /// avoids clobbering a developer’s real `~/.codex`. pub async fn load_default_config_for_test(codex_home: &TempDir) -> Config { + load_default_config_for_test_with_cloud_requirements( + codex_home, + CloudRequirementsLoader::default(), + ) + .await +} + +/// Returns a default `Config` with test-provided cloud requirements applied +/// during config construction. +pub async fn load_default_config_for_test_with_cloud_requirements( + codex_home: &TempDir, + cloud_requirements: CloudRequirementsLoader, +) -> Config { ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .harness_overrides(default_test_overrides()) + .cloud_requirements(cloud_requirements) .build() .await .expect("defaults for test should always succeed") } +pub fn managed_network_requirements_loader() -> CloudRequirementsLoader { + CloudRequirementsLoader::new(async { + Ok(Some(ConfigRequirementsToml { + network: Some(NetworkRequirementsToml { + enabled: Some(true), + allow_local_binding: Some(true), + ..Default::default() + }), + ..Default::default() + })) + }) +} + #[cfg(target_os = "linux")] fn default_test_overrides() -> ConfigOverrides { ConfigOverrides { diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 9a010417fb5f..d77d55956a63 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -12,6 +12,7 @@ use std::time::Duration; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; +use codex_config::CloudRequirementsLoader; use codex_core::CodexThread; use codex_core::ThreadManager; use codex_core::config::Config; @@ -47,6 +48,7 @@ use crate::PathBufExt; use crate::TempDirExt; use crate::get_remote_test_env; use crate::load_default_config_for_test; +use crate::load_default_config_for_test_with_cloud_requirements; use crate::responses::WebSocketTestServer; use crate::responses::output_value_to_text; use crate::responses::start_mock_server; @@ -206,6 +208,7 @@ pub struct TestCodexBuilder { pre_build_hooks: Vec>, workspace_setups: Vec>, home: Option>, + cloud_requirements: Option, user_shell_override: Option, exec_server_url: Option, } @@ -254,6 +257,11 @@ impl TestCodexBuilder { self } + pub fn with_cloud_requirements(mut self, cloud_requirements: CloudRequirementsLoader) -> Self { + self.cloud_requirements = Some(cloud_requirements); + self + } + pub fn with_user_shell(mut self, user_shell: Shell) -> Self { self.user_shell_override = Some(user_shell); self @@ -485,7 +493,11 @@ impl TestCodexBuilder { for hook in self.pre_build_hooks.drain(..) { hook(home.path()); } - let mut config = load_default_config_for_test(home).await; + let mut config = if let Some(cloud_requirements) = self.cloud_requirements.take() { + load_default_config_for_test_with_cloud_requirements(home, cloud_requirements).await + } else { + load_default_config_for_test(home).await + }; config.cwd = cwd_override; config.model_provider = model_provider; if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { @@ -923,6 +935,7 @@ pub fn test_codex() -> TestCodexBuilder { pre_build_hooks: vec![], workspace_setups: vec![], home: None, + cloud_requirements: None, user_shell_override: None, exec_server_url: None, } diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 4209fc2100d5..ef22cfbfc1a2 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -2,12 +2,6 @@ use anyhow::Context; use anyhow::Result; -use codex_config::ConfigLayerStack; -use codex_config::ConfigLayerStackOrdering; -use codex_config::NetworkConstraints; -use codex_config::NetworkRequirementsToml; -use codex_config::RequirementSource; -use codex_config::Sourced; use codex_config::types::ApprovalsReviewer; use codex_core::CodexThread; use codex_core::config::Constrained; @@ -25,6 +19,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +use core_test_support::managed_network_requirements_loader; use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -2590,38 +2585,15 @@ allow_local_binding = true exclude_slash_tmp: false, }; let sandbox_policy_for_config = sandbox_policy.clone(); - let mut builder = test_codex().with_home(home).with_config(move |config| { - config.permissions.approval_policy = Constrained::allow_any(approval_policy); - config - .set_legacy_sandbox_policy(sandbox_policy_for_config) - .expect("set sandbox policy"); - let layers = config - .config_layer_stack - .get_layers( - ConfigLayerStackOrdering::LowestPrecedenceFirst, - /*include_disabled*/ true, - ) - .into_iter() - .cloned() - .collect(); - let mut requirements = config.config_layer_stack.requirements().clone(); - requirements.network = Some(Sourced::new( - NetworkConstraints { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() - }, - RequirementSource::CloudRequirements, - )); - let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); - requirements_toml.network = Some(NetworkRequirementsToml { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() + let mut builder = test_codex() + .with_home(home) + .with_cloud_requirements(managed_network_requirements_loader()) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config + .set_legacy_sandbox_policy(sandbox_policy_for_config) + .expect("set sandbox policy"); }); - config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml) - .expect("rebuild config layer stack with network requirements"); - }); let test = builder.build(&server).await?; assert!( test.config.managed_network_requirements_enabled(), @@ -2892,40 +2864,17 @@ allow_local_binding = true exclude_tmpdir_env_var: false, exclude_slash_tmp: false, }; - let mut builder = test_codex().with_home(home).with_config(move |config| { - config.permissions.approval_policy = Constrained::allow_any(approval_policy); - let cwd = config.cwd.clone(); - config - .permissions - .set_legacy_sandbox_policy(SandboxPolicy::DangerFullAccess, cwd.as_path()) - .expect("test setup should allow sandbox policy"); - let layers = config - .config_layer_stack - .get_layers( - ConfigLayerStackOrdering::LowestPrecedenceFirst, - /*include_disabled*/ true, - ) - .into_iter() - .cloned() - .collect(); - let mut requirements = config.config_layer_stack.requirements().clone(); - requirements.network = Some(Sourced::new( - NetworkConstraints { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() - }, - RequirementSource::CloudRequirements, - )); - let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); - requirements_toml.network = Some(NetworkRequirementsToml { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() + let mut builder = test_codex() + .with_home(home) + .with_cloud_requirements(managed_network_requirements_loader()) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + let cwd = config.cwd.clone(); + config + .permissions + .set_legacy_sandbox_policy(SandboxPolicy::DangerFullAccess, cwd.as_path()) + .expect("test setup should allow sandbox policy"); }); - config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml) - .expect("rebuild config layer stack with network requirements"); - }); let test = builder.build(&server).await?; assert!( !test.config.managed_network_requirements_enabled(), diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 74e9a7a6824d..28185a0a5c81 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -3,12 +3,6 @@ use std::path::Path; use anyhow::Context; use anyhow::Result; -use codex_config::ConfigLayerStack; -use codex_config::ConfigLayerStackOrdering; -use codex_config::NetworkConstraints; -use codex_config::NetworkRequirementsToml; -use codex_config::RequirementSource; -use codex_config::Sourced; use codex_core::config::Constrained; use codex_features::Feature; use codex_protocol::items::parse_hook_prompt_fragment; @@ -21,6 +15,7 @@ use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +use core_test_support::managed_network_requirements_loader; use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -1577,6 +1572,7 @@ allow_local_binding = true panic!("failed to write permission request hook test fixture: {error}"); } }) + .with_cloud_requirements(managed_network_requirements_loader()) .with_config(move |config| { config .features @@ -1586,33 +1582,6 @@ allow_local_binding = true config .set_legacy_sandbox_policy(sandbox_policy_for_config) .expect("set sandbox policy"); - let layers = config - .config_layer_stack - .get_layers( - ConfigLayerStackOrdering::LowestPrecedenceFirst, - /*include_disabled*/ true, - ) - .into_iter() - .cloned() - .collect(); - let mut requirements = config.config_layer_stack.requirements().clone(); - requirements.network = Some(Sourced::new( - NetworkConstraints { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() - }, - RequirementSource::CloudRequirements, - )); - let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); - requirements_toml.network = Some(NetworkRequirementsToml { - enabled: Some(true), - allow_local_binding: Some(true), - ..Default::default() - }); - config.config_layer_stack = - ConfigLayerStack::new(layers, requirements, requirements_toml) - .expect("rebuild config layer stack with network requirements"); }) .build(&server) .await?;