diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 84ee3d5c1f..509d442678 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1637,6 +1637,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 85de9c0dab..a22962cd40 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -5350,6 +5350,10 @@ ], "description": "Read access granted while running under this policy." }, + "network_access": { + "description": "When set to `true`, outbound network access is allowed. `false` by default.", + "type": "boolean" + }, "type": { "enum": [ "read-only" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index ac832cd93b..374bdc1971 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -11972,6 +11972,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 0d11718a88..2ca096cff1 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -9752,6 +9752,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json b/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json index 4528b34159..08f1a9a15a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json @@ -89,6 +89,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index 3e4dca5b1c..41f4c9b7bb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index d9fbc0d02c..416bceb6e0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index ee8b384998..25db79cf24 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index 0a769c3525..9e749d5b81 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -214,6 +214,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts index 743ad22229..8440fd8043 100644 --- a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts @@ -12,7 +12,12 @@ export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-o /** * Read access granted while running under this policy. */ -access?: ReadOnlyAccess, } | { "type": "external-sandbox", +access?: ReadOnlyAccess, +/** + * When set to `true`, outbound network access is allowed. `false` by + * default. + */ +network_access?: boolean, } | { "type": "external-sandbox", /** * Whether the external sandbox permits outbound network traffic. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts index c81c2642d2..c6780648cf 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts @@ -5,4 +5,4 @@ import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { NetworkAccess } from "./NetworkAccess"; import type { ReadOnlyAccess } from "./ReadOnlyAccess"; -export type SandboxPolicy = { "type": "dangerFullAccess" } | { "type": "readOnly", access: ReadOnlyAccess, } | { "type": "externalSandbox", networkAccess: NetworkAccess, } | { "type": "workspaceWrite", writableRoots: Array, readOnlyAccess: ReadOnlyAccess, networkAccess: boolean, excludeTmpdirEnvVar: boolean, excludeSlashTmp: boolean, }; +export type SandboxPolicy = { "type": "dangerFullAccess" } | { "type": "readOnly", access: ReadOnlyAccess, networkAccess: boolean, } | { "type": "externalSandbox", networkAccess: NetworkAccess, } | { "type": "workspaceWrite", writableRoots: Array, readOnlyAccess: ReadOnlyAccess, networkAccess: boolean, excludeTmpdirEnvVar: boolean, excludeSlashTmp: boolean, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index a5eca293ac..7c69d9fa11 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -952,6 +952,8 @@ pub enum SandboxPolicy { ReadOnly { #[serde(default)] access: ReadOnlyAccess, + #[serde(default)] + network_access: bool, }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] @@ -981,11 +983,13 @@ impl SandboxPolicy { SandboxPolicy::DangerFullAccess => { codex_protocol::protocol::SandboxPolicy::DangerFullAccess } - SandboxPolicy::ReadOnly { access } => { - codex_protocol::protocol::SandboxPolicy::ReadOnly { - access: access.to_core(), - } - } + SandboxPolicy::ReadOnly { + access, + network_access, + } => codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: access.to_core(), + network_access: *network_access, + }, SandboxPolicy::ExternalSandbox { network_access } => { codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -1017,11 +1021,13 @@ impl From for SandboxPolicy { codex_protocol::protocol::SandboxPolicy::DangerFullAccess => { SandboxPolicy::DangerFullAccess } - codex_protocol::protocol::SandboxPolicy::ReadOnly { access } => { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::from(access), - } - } + codex_protocol::protocol::SandboxPolicy::ReadOnly { + access, + network_access, + } => SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::from(access), + network_access, + }, codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access } => { SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -4344,6 +4350,7 @@ mod tests { include_platform_defaults: false, readable_roots: vec![readable_root.clone()], }, + network_access: true, }; let core_policy = v2_policy.to_core(); @@ -4354,6 +4361,7 @@ mod tests { include_platform_defaults: false, readable_roots: vec![readable_root], }, + network_access: true, } ); @@ -4404,6 +4412,7 @@ mod tests { policy, SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, } ); } diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 7baad9be66..06f677ceb7 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -589,6 +589,7 @@ fn trigger_zsh_fork_multi_cmd_approval( turn_params.approval_policy = Some(AskForApproval::OnRequest); turn_params.sandbox_policy = Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }); let turn_response = client.turn_start(turn_params)?; @@ -722,6 +723,7 @@ fn trigger_cmd_approval( Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }), dynamic_tools, ) @@ -744,6 +746,7 @@ fn trigger_patch_approval( Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }), dynamic_tools, ) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 749f6de23b..71d624d2c0 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -20,6 +20,13 @@ "description": { "description": "Human-facing role documentation used in spawn tool guidance.", "type": "string" + }, + "nickname_candidates": { + "description": "Candidate nicknames for agents spawned with this role.", + "items": { + "type": "string" + }, + "type": "array" } }, "type": "object" diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index fa643ff3ce..e7547f9c9d 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -1,5 +1,7 @@ use crate::agent::AgentStatus; use crate::agent::guards::Guards; +use crate::agent::role::DEFAULT_ROLE_NAME; +use crate::agent::role::resolve_role_config; use crate::agent::status::is_final; use crate::error::CodexErr; use crate::error::Result as CodexResult; @@ -32,7 +34,7 @@ pub(crate) struct SpawnAgentOptions { pub(crate) fork_parent_spawn_call_id: Option, } -fn agent_nickname_list() -> Vec<&'static str> { +fn default_agent_nickname_list() -> Vec<&'static str> { AGENT_NAMES .lines() .map(str::trim) @@ -40,6 +42,23 @@ fn agent_nickname_list() -> Vec<&'static str> { .collect() } +fn agent_nickname_candidates( + config: &crate::config::Config, + role_name: Option<&str>, +) -> Vec { + let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); + if let Some(candidates) = + resolve_role_config(config, role_name).and_then(|role| role.nickname_candidates.clone()) + { + return candidates; + } + + default_agent_nickname_list() + .into_iter() + .map(ToOwned::to_owned) + .collect() +} + /// Control-plane handle for multi-agent operations. /// `AgentControl` is held by each session (via `SessionServices`). It provides capability to /// spawn new agents and the inter-agent communication layer. @@ -94,7 +113,10 @@ impl AgentControl { agent_role, .. })) => { - let agent_nickname = reservation.reserve_agent_nickname(&agent_nickname_list())?; + let candidate_names = agent_nickname_candidates(&config, agent_role.as_deref()); + let candidate_name_refs: Vec<&str> = + candidate_names.iter().map(String::as_str).collect(); + let agent_nickname = reservation.reserve_agent_nickname(&candidate_name_refs)?; Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth, @@ -225,8 +247,12 @@ impl AgentControl { let reserved_agent_nickname = resumed_agent_nickname .as_deref() .map(|agent_nickname| { + let candidate_names = + agent_nickname_candidates(&config, resumed_agent_role.as_deref()); + let candidate_name_refs: Vec<&str> = + candidate_names.iter().map(String::as_str).collect(); reservation.reserve_agent_nickname_with_preference( - &agent_nickname_list(), + &candidate_name_refs, Some(agent_nickname), ) }) @@ -467,6 +493,7 @@ mod tests { use crate::CodexThread; use crate::ThreadManager; use crate::agent::agent_status_from_event; + use crate::config::AgentRoleConfig; use crate::config::Config; use crate::config::ConfigBuilder; use crate::config_loader::LoaderOverrides; @@ -1378,6 +1405,49 @@ mod tests { assert_eq!(agent_role, Some("explorer".to_string())); } + #[tokio::test] + async fn spawn_thread_subagent_uses_role_specific_nickname_candidates() { + let mut harness = AgentControlHarness::new().await; + harness.config.agent_roles.insert( + "researcher".to_string(), + AgentRoleConfig { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec!["Atlas".to_string()]), + }, + ); + let (parent_thread_id, _parent_thread) = harness.start_thread().await; + + let child_thread_id = harness + .control + .spawn_agent( + harness.config.clone(), + text_input("hello child"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id, + depth: 1, + agent_nickname: None, + agent_role: Some("researcher".to_string()), + })), + ) + .await + .expect("child spawn should succeed"); + + let child_thread = harness + .manager + .get_thread(child_thread_id) + .await + .expect("child thread should be registered"); + let snapshot = child_thread.config_snapshot().await; + + let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { agent_nickname, .. }) = + snapshot.session_source + else { + panic!("expected thread-spawn sub-agent source"); + }; + assert_eq!(agent_nickname, Some("Atlas".to_string())); + } + #[tokio::test] async fn resume_thread_subagent_restores_stored_nickname_and_role() { let (home, mut config) = test_config().await; diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 3ca496d6e1..0793febe54 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -38,15 +38,9 @@ pub(crate) async fn apply_role_to_config( role_name: Option<&str>, ) -> Result<(), String> { let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); - let (config_file, is_built_in) = config - .agent_roles - .get(role_name) - .map(|role| (&role.config_file, false)) - .or_else(|| { - built_in::configs() - .get(role_name) - .map(|role| (&role.config_file, true)) - }) + let is_built_in = !config.agent_roles.contains_key(role_name); + let (config_file, is_built_in) = resolve_role_config(config, role_name) + .map(|role| (&role.config_file, is_built_in)) .ok_or_else(|| format!("unknown agent_type '{role_name}'"))?; let Some(config_file) = config_file.as_ref() else { return Ok(()); @@ -139,6 +133,16 @@ pub(crate) async fn apply_role_to_config( Ok(()) } +pub(crate) fn resolve_role_config<'a>( + config: &'a Config, + role_name: &str, +) -> Option<&'a AgentRoleConfig> { + config + .agent_roles + .get(role_name) + .or_else(|| built_in::configs().get(role_name)) +} + pub(crate) mod spawn_tool_spec { use super::*; @@ -196,6 +200,7 @@ mod built_in { AgentRoleConfig { description: Some("Default agent.".to_string()), config_file: None, + nickname_candidates: None, } ), ( @@ -210,6 +215,7 @@ Rules: - Run explorers in parallel when useful. - Reuse existing explorers for related questions."#.to_string()), config_file: Some("explorer.toml".to_string().parse().unwrap_or_default()), + nickname_candidates: None, } ), ( @@ -224,6 +230,7 @@ Rules: - Explicitly assign **ownership** of the task (files / responsibility). - Always tell workers they are **not alone in the codebase**, and they should ignore edits made by others without touching them."#.to_string()), config_file: None, + nickname_candidates: None, } ), // Awaiter is temp removed @@ -354,6 +361,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(PathBuf::from("/path/does/not/exist.toml")), + nickname_candidates: None, }, ); @@ -373,6 +381,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -403,6 +412,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -456,6 +466,7 @@ model_provider = "test-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -512,6 +523,7 @@ model_provider = "role-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -569,6 +581,7 @@ model_provider = "base-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -630,6 +643,7 @@ model_reasoning_effort = "high" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -671,6 +685,7 @@ writable_roots = ["./sandbox-root"] AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -724,6 +739,7 @@ writable_roots = ["./sandbox-root"] AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -764,6 +780,7 @@ enabled = false AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -791,6 +808,7 @@ enabled = false AgentRoleConfig { description: Some("user override".to_string()), config_file: None, + nickname_candidates: None, }, ), ("researcher".to_string(), AgentRoleConfig::default()), @@ -811,6 +829,7 @@ enabled = false AgentRoleConfig { description: Some("first".to_string()), config_file: None, + nickname_candidates: None, }, )]); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3b71f8fac4..2d833d8b51 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -78,6 +78,7 @@ use serde::Deserialize; use serde::Serialize; use similar::DiffableStr; use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashMap; use std::io::ErrorKind; use std::path::Path; @@ -1385,6 +1386,7 @@ pub struct AgentsToml { /// [agents.researcher] /// description = "Research-focused role." /// config_file = "./agents/researcher.toml" + /// nickname_candidates = ["Herodotus", "Ibn Battuta"] /// ``` #[serde(default, flatten)] pub roles: BTreeMap, @@ -1396,6 +1398,8 @@ pub struct AgentRoleConfig { pub description: Option, /// Path to a role-specific config layer. pub config_file: Option, + /// Candidate nicknames for agents spawned with this role. + pub nickname_candidates: Option>, } #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] @@ -1407,6 +1411,9 @@ pub struct AgentRoleToml { /// Path to a role-specific config layer. /// Relative paths are resolved relative to the `config.toml` that defines them. pub config_file: Option, + + /// Candidate nicknames for agents spawned with this role. + pub nickname_candidates: Option>, } impl From for Tools { @@ -1895,11 +1902,16 @@ impl Config { let config_file = role.config_file.as_ref().map(AbsolutePathBuf::to_path_buf); Self::validate_agent_role_config_file(name, config_file.as_deref())?; + let nickname_candidates = Self::normalize_agent_role_nickname_candidates( + name, + role.nickname_candidates.as_deref(), + )?; Ok(( name.clone(), AgentRoleConfig { description: role.description.clone(), config_file, + nickname_candidates, }, )) }) @@ -2361,6 +2373,58 @@ impl Config { } } + fn normalize_agent_role_nickname_candidates( + role_name: &str, + nickname_candidates: Option<&[String]>, + ) -> std::io::Result>> { + let Some(nickname_candidates) = nickname_candidates else { + return Ok(None); + }; + + if nickname_candidates.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates must contain at least one name"), + )); + } + + let mut normalized_candidates = Vec::with_capacity(nickname_candidates.len()); + let mut seen_candidates = BTreeSet::new(); + + for nickname in nickname_candidates { + let normalized_nickname = nickname.trim(); + if normalized_nickname.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates cannot contain blank names"), + )); + } + + if !seen_candidates.insert(normalized_nickname.to_owned()) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates cannot contain duplicates"), + )); + } + + if !normalized_nickname + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, ' ' | '-' | '_')) + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "agents.{role_name}.nickname_candidates may only contain ASCII letters, digits, spaces, hyphens, and underscores" + ), + )); + } + + normalized_candidates.push(normalized_nickname.to_owned()); + } + + Ok(Some(normalized_candidates)) + } + pub fn set_windows_sandbox_enabled(&mut self, value: bool) { self.permissions.windows_sandbox_mode = if value { Some(WindowsSandboxModeToml::Unelevated) @@ -4662,6 +4726,7 @@ model = "gpt-5.1-codex" AgentRoleToml { description: Some("Research role".to_string()), config_file: Some(AbsolutePathBuf::from_absolute_path(missing_path)?), + nickname_candidates: None, }, )]), }), @@ -4698,6 +4763,7 @@ model = "gpt-5.1-codex" r#"[agents.researcher] description = "Research role" config_file = "./agents/researcher.toml" +nickname_candidates = ["Hypatia", "Noether"] "#, ) .await?; @@ -4714,6 +4780,162 @@ config_file = "./agents/researcher.toml" .and_then(|role| role.config_file.as_ref()), Some(&role_config_path) ); + assert_eq!( + config + .agent_roles + .get("researcher") + .and_then(|role| role.nickname_candidates.as_ref()) + .map(|candidates| candidates.iter().map(String::as_str).collect::>()), + Some(vec!["Hypatia", "Noether"]) + ); + + Ok(()) + } + + #[test] + fn load_config_normalizes_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec![ + " Hypatia ".to_string(), + "Noether".to_string(), + ]), + }, + )]), + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config + .agent_roles + .get("researcher") + .and_then(|role| role.nickname_candidates.as_ref()) + .map(|candidates| candidates.iter().map(String::as_str).collect::>()), + Some(vec!["Hypatia", "Noether"]) + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_empty_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(Vec::new()), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("empty nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.to_string() + .contains("agents.researcher.nickname_candidates") + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_duplicate_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec![ + "Hypatia".to_string(), + " Hypatia ".to_string(), + ]), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("duplicate nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.to_string() + .contains("agents.researcher.nickname_candidates cannot contain duplicates") + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_unsafe_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec!["Agent ".to_string()]), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("unsafe nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!(err.to_string().contains( + "agents.researcher.nickname_candidates may only contain ASCII letters, digits, spaces, hyphens, and underscores" + )); Ok(()) } diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index bbd73749e4..fe7d077bb8 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -31,6 +31,7 @@ use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; +use dunce::canonicalize; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; @@ -110,6 +111,7 @@ pub(crate) fn normalize_additional_permissions( .write .map(|paths| normalize_permission_paths(paths, "file_system.write")); Ok(PermissionProfile { + network: additional_permissions.network, file_system: Some(FileSystemPermissions { read, write }), ..Default::default() }) @@ -123,9 +125,7 @@ fn normalize_permission_paths( let mut seen = HashSet::new(); for path in paths { - let canonicalized = path - .as_path() - .canonicalize() + let canonicalized = canonicalize(path.as_path()) .ok() .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) .unwrap_or(path); @@ -189,6 +189,13 @@ fn merge_read_only_access_with_additional_reads( } } +fn merge_network_access( + base_network_access: bool, + additional_permissions: &PermissionProfile, +) -> bool { + base_network_access || matches!(additional_permissions.network, Some(true)) +} + fn sandbox_policy_with_additional_permissions( sandbox_policy: &SandboxPolicy, additional_permissions: &PermissionProfile, @@ -218,15 +225,19 @@ fn sandbox_policy_with_additional_permissions( read_only_access, extra_reads, ), - network_access: *network_access, + network_access: merge_network_access(*network_access, additional_permissions), exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, } } - SandboxPolicy::ReadOnly { access } => { + SandboxPolicy::ReadOnly { + access, + network_access, + } => { if extra_writes.is_empty() { SandboxPolicy::ReadOnly { access: merge_read_only_access_with_additional_reads(access, extra_reads), + network_access: merge_network_access(*network_access, additional_permissions), } } else { // todo(dylan) - for now, this grants more access than the request. We should restrict this, @@ -238,7 +249,7 @@ fn sandbox_policy_with_additional_permissions( access, extra_reads, ), - network_access: false, + network_access: merge_network_access(*network_access, additional_permissions), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } @@ -412,11 +423,19 @@ pub async fn execute_env( #[cfg(test)] mod tests { use super::SandboxManager; + use super::normalize_additional_permissions; + use super::sandbox_policy_with_additional_permissions; use crate::exec::SandboxType; + use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::tools::sandboxing::SandboxablePreference; use codex_protocol::config_types::WindowsSandboxLevel; + use codex_protocol::models::FileSystemPermissions; + use codex_protocol::models::PermissionProfile; + use codex_utils_absolute_path::AbsolutePathBuf; + use dunce::canonicalize; use pretty_assertions::assert_eq; + use tempfile::TempDir; #[test] fn danger_full_access_defaults_to_no_sandbox_without_network_requirements() { @@ -442,4 +461,69 @@ mod tests { ); assert_eq!(sandbox, expected); } + + #[test] + fn normalize_additional_permissions_preserves_network() { + let temp_dir = TempDir::new().expect("create temp dir"); + let path = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let permissions = normalize_additional_permissions(PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![path.clone()]), + write: Some(vec![path.clone()]), + }), + ..Default::default() + }) + .expect("permissions"); + + assert_eq!(permissions.network, Some(true)); + assert_eq!( + permissions.file_system, + Some(FileSystemPermissions { + read: Some(vec![path.clone()]), + write: Some(vec![path]), + }) + ); + } + + #[test] + fn read_only_additional_permissions_can_enable_network_without_writes() { + let temp_dir = TempDir::new().expect("create temp dir"); + let path = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let policy = sandbox_policy_with_additional_permissions( + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![path.clone()], + }, + network_access: false, + }, + &PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![path.clone()]), + write: Some(Vec::new()), + }), + ..Default::default() + }, + ) + .expect("policy"); + + assert_eq!( + policy, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![path], + }, + network_access: true, + } + ); + } } diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 1d0e6c82ad..825128307d 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -44,6 +44,7 @@ pub(crate) fn compile_permission_profile( file_system, macos, } = permissions?; + let network_access = network.unwrap_or_default(); let file_system = file_system.unwrap_or_default(); let fs_read = normalize_permission_paths( file_system.read.as_deref().unwrap_or_default(), @@ -64,7 +65,7 @@ pub(crate) fn compile_permission_profile( readable_roots: fs_read, } }, - network_access: network.unwrap_or_default(), + network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } @@ -74,6 +75,12 @@ pub(crate) fn compile_permission_profile( include_platform_defaults: true, readable_roots: fs_read, }, + network_access, + } + } else if network_access { + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, } } else { // Default sandbox policy @@ -320,7 +327,10 @@ mod tests { profile, Permissions { approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()), + sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, + }), network: None, allow_login_shell: true, shell_environment_policy: ShellEnvironmentPolicy::default(), @@ -366,6 +376,7 @@ mod tests { .expect("absolute read path") ], }, + network_access: true, }), network: None, allow_login_shell: true, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 101d9309fd..c4d15273a6 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -5,7 +5,7 @@ Executes shell requests under the orchestrator: asks for approval when needed, builds a CommandSpec, and runs it under the current SandboxAttempt. */ #[cfg(unix)] -mod unix_escalation; +pub(crate) mod unix_escalation; use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecToolCallOutput; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 36f63be4fa..4fc22209d9 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -6,6 +6,7 @@ use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; use crate::exec::is_likely_sandbox_denied; use crate::features::Feature; +use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; use crate::skills::SkillMetadata; @@ -36,6 +37,7 @@ use codex_shell_escalation::EscalationDecision; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::EscalationPolicy; +use codex_shell_escalation::EscalationSession; use codex_shell_escalation::ExecParams; use codex_shell_escalation::ExecResult; use codex_shell_escalation::Permissions as EscalatedPermissions; @@ -51,6 +53,11 @@ use tokio::sync::RwLock; use tokio_util::sync::CancellationToken; use uuid::Uuid; +pub(crate) struct PreparedUnifiedExecZshFork { + pub(crate) exec_request: ExecRequest, + pub(crate) escalation_session: EscalationSession, +} + pub(super) async fn try_run_zsh_fork( req: &ShellRequest, attempt: &SandboxAttempt<'_>, @@ -95,7 +102,7 @@ pub(super) async fn try_run_zsh_fork( justification, arg0, } = sandbox_exec_request; - let ParsedShellCommand { script, login } = extract_shell_script(&command)?; + let ParsedShellCommand { script, login, .. } = extract_shell_script(&command)?; let effective_timeout = Duration::from_millis( req.timeout_ms .unwrap_or(crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS), @@ -172,6 +179,103 @@ pub(super) async fn try_run_zsh_fork( map_exec_result(attempt.sandbox, exec_result).map(Some) } +pub(crate) async fn prepare_unified_exec_zsh_fork( + req: &crate::tools::runtimes::unified_exec::UnifiedExecRequest, + attempt: &SandboxAttempt<'_>, + ctx: &ToolCtx, + exec_request: ExecRequest, +) -> Result, ToolError> { + let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else { + tracing::warn!("ZshFork backend specified, but shell_zsh_path is not configured."); + return Ok(None); + }; + if !ctx.session.features().enabled(Feature::ShellZshFork) { + tracing::warn!("ZshFork backend specified, but ShellZshFork feature is not enabled."); + return Ok(None); + } + if !matches!(ctx.session.user_shell().shell_type, ShellType::Zsh) { + tracing::warn!("ZshFork backend specified, but user shell is not Zsh."); + return Ok(None); + } + + let parsed = match extract_shell_script(&exec_request.command) { + Ok(parsed) => parsed, + Err(err) => { + tracing::warn!("ZshFork unified exec fallback: {err:?}"); + return Ok(None); + } + }; + if parsed.program != shell_zsh_path.to_string_lossy() { + tracing::warn!( + "ZshFork backend specified, but unified exec command targets `{}` instead of `{}`.", + parsed.program, + shell_zsh_path.display(), + ); + return Ok(None); + } + + let exec_policy = Arc::new(RwLock::new( + ctx.session.services.exec_policy.current().as_ref().clone(), + )); + let command_executor = CoreShellCommandExecutor { + command: exec_request.command.clone(), + cwd: exec_request.cwd.clone(), + sandbox_policy: exec_request.sandbox_policy.clone(), + sandbox: exec_request.sandbox, + env: exec_request.env.clone(), + network: exec_request.network.clone(), + windows_sandbox_level: exec_request.windows_sandbox_level, + sandbox_permissions: exec_request.sandbox_permissions, + justification: exec_request.justification.clone(), + arg0: exec_request.arg0.clone(), + sandbox_policy_cwd: ctx.turn.cwd.clone(), + macos_seatbelt_profile_extensions: ctx + .turn + .config + .permissions + .macos_seatbelt_profile_extensions + .clone(), + codex_linux_sandbox_exe: ctx.turn.codex_linux_sandbox_exe.clone(), + use_linux_sandbox_bwrap: ctx.turn.features.enabled(Feature::UseLinuxSandboxBwrap), + }; + let main_execve_wrapper_exe = ctx + .session + .services + .main_execve_wrapper_exe + .clone() + .ok_or_else(|| { + ToolError::Rejected( + "zsh fork feature enabled, but execve wrapper is not configured".to_string(), + ) + })?; + let escalation_policy = CoreShellActionProvider { + policy: Arc::clone(&exec_policy), + session: Arc::clone(&ctx.session), + turn: Arc::clone(&ctx.turn), + call_id: ctx.call_id.clone(), + approval_policy: ctx.turn.approval_policy.value(), + sandbox_policy: attempt.policy.clone(), + sandbox_permissions: req.sandbox_permissions, + prompt_permissions: req.additional_permissions.clone(), + stopwatch: Stopwatch::unlimited(), + }; + + let escalate_server = EscalateServer::new( + shell_zsh_path.clone(), + main_execve_wrapper_exe, + escalation_policy, + ); + let escalation_session = escalate_server + .start_session(Arc::new(command_executor)) + .map_err(|err| ToolError::Rejected(err.to_string()))?; + let mut exec_request = exec_request; + exec_request.env.extend(escalation_session.env().clone()); + Ok(Some(PreparedUnifiedExecZshFork { + exec_request, + escalation_session, + })) +} + struct CoreShellActionProvider { policy: Arc>, session: Arc, @@ -809,6 +913,7 @@ impl CoreShellCommandExecutor { #[derive(Debug, Eq, PartialEq)] struct ParsedShellCommand { + program: String, script: String, login: bool, } @@ -817,12 +922,20 @@ fn extract_shell_script(command: &[String]) -> Result Some((script.to_owned(), false)), - [_, flag, script] if flag == "-lc" => Some((script.to_owned(), true)), + if let Some((program, script, login)) = command.windows(3).find_map(|parts| match parts { + [program, flag, script] if flag == "-c" => { + Some((program.to_owned(), script.to_owned(), false)) + } + [program, flag, script] if flag == "-lc" => { + Some((program.to_owned(), script.to_owned(), true)) + } _ => None, }) { - return Ok(ParsedShellCommand { script, login }); + return Ok(ParsedShellCommand { + program, + script, + login, + }); } Err(ToolError::Rejected( diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index b88f5b0542..ad663a3fed 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -64,6 +64,7 @@ fn extract_shell_script_preserves_login_flag() { assert_eq!( extract_shell_script(&["/bin/zsh".into(), "-lc".into(), "echo hi".into()]).unwrap(), ParsedShellCommand { + program: "/bin/zsh".to_string(), script: "echo hi".to_string(), login: true, } @@ -71,6 +72,7 @@ fn extract_shell_script_preserves_login_flag() { assert_eq!( extract_shell_script(&["/bin/zsh".into(), "-c".into(), "echo hi".into()]).unwrap(), ParsedShellCommand { + program: "/bin/zsh".to_string(), script: "echo hi".to_string(), login: false, } @@ -89,6 +91,7 @@ fn extract_shell_script_supports_wrapped_command_prefixes() { ]) .unwrap(), ParsedShellCommand { + program: "/bin/zsh".to_string(), script: "echo hello".to_string(), login: true, } @@ -105,6 +108,7 @@ fn extract_shell_script_supports_wrapped_command_prefixes() { ]) .unwrap(), ParsedShellCommand { + program: "/bin/zsh".to_string(), script: "pwd".to_string(), login: false, } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index f4d823bf0e..71f5645873 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -16,6 +16,8 @@ use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; +#[cfg(unix)] +use crate::tools::runtimes::shell::unix_escalation; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -28,6 +30,7 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::with_cached_approval; +use crate::tools::spec::UnifiedExecBackendConfig; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; @@ -63,10 +66,18 @@ pub struct UnifiedExecApprovalKey { pub struct UnifiedExecRuntime<'a> { manager: &'a UnifiedExecProcessManager, + #[cfg(unix)] + backend: UnifiedExecBackendConfig, } impl<'a> UnifiedExecRuntime<'a> { - pub fn new(manager: &'a UnifiedExecProcessManager) -> Self { + #[cfg(unix)] + pub fn new(manager: &'a UnifiedExecProcessManager, backend: UnifiedExecBackendConfig) -> Self { + Self { manager, backend } + } + + #[cfg(not(unix))] + pub fn new(manager: &'a UnifiedExecProcessManager, _backend: UnifiedExecBackendConfig) -> Self { Self { manager } } } @@ -184,6 +195,54 @@ impl<'a> ToolRuntime for UnifiedExecRunt if let Some(network) = req.network.as_ref() { network.apply_to_env(&mut env); } + #[cfg(unix)] + if self.backend == UnifiedExecBackendConfig::ZshFork { + let spec = build_command_spec( + &command, + &req.cwd, + &env, + ExecExpiration::DefaultTimeout, + req.sandbox_permissions, + req.additional_permissions.clone(), + req.justification.clone(), + ) + .map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?; + let exec_env = attempt + .env_for(spec, req.network.as_ref()) + .map_err(|err| ToolError::Codex(err.into()))?; + match unix_escalation::prepare_unified_exec_zsh_fork(req, attempt, ctx, exec_env) + .await? + { + Some(prepared) => { + let unix_escalation::PreparedUnifiedExecZshFork { + exec_request, + escalation_session, + } = prepared; + return self + .manager + .open_session_with_exec_env( + &exec_request, + req.tty, + Some(escalation_session), + ) + .await + .map_err(|err| match err { + UnifiedExecError::SandboxDenied { output, .. } => { + ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + network_policy_decision: None, + })) + } + other => ToolError::Rejected(other.to_string()), + }); + } + None => { + tracing::warn!( + "UnifiedExec ZshFork backend specified, but conditions for using it were not met, falling back to direct execution", + ); + } + } + } let spec = build_command_spec( &command, &req.cwd, @@ -198,7 +257,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt .env_for(spec, req.network.as_ref()) .map_err(|err| ToolError::Codex(err.into()))?; self.manager - .open_session_with_exec_env(&exec_env, req.tty) + .open_session_with_exec_env(&exec_env, req.tty, None) .await .map_err(|err| match err { UnifiedExecError::SandboxDenied { output, .. } => { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 0e53670750..2e1dacfa9a 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -42,10 +42,17 @@ pub enum ShellCommandBackendConfig { ZshFork, } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum UnifiedExecBackendConfig { + Direct, + ZshFork, +} + #[derive(Debug, Clone)] pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, shell_command_backend: ShellCommandBackendConfig, + pub unified_exec_backend: UnifiedExecBackendConfig, pub allow_login_shell: bool, pub apply_patch_tool_type: Option, pub web_search_mode: Option, @@ -94,6 +101,12 @@ impl ToolsConfig { } else { ShellCommandBackendConfig::Classic }; + let unified_exec_backend = + if features.enabled(Feature::ShellTool) && features.enabled(Feature::ShellZshFork) { + UnifiedExecBackendConfig::ZshFork + } else { + UnifiedExecBackendConfig::Direct + }; let shell_type = if !features.enabled(Feature::ShellTool) { ConfigShellToolType::Disabled @@ -132,6 +145,7 @@ impl ToolsConfig { Self { shell_type, shell_command_backend, + unified_exec_backend, allow_login_shell: true, apply_patch_tool_type, web_search_mode: *web_search_mode, @@ -2727,6 +2741,10 @@ mod tests { tools_config.shell_command_backend, ShellCommandBackendConfig::ZshFork ); + assert_eq!( + tools_config.unified_exec_backend, + UnifiedExecBackendConfig::ZshFork + ); } #[test] diff --git a/codex-rs/core/src/unified_exec/process.rs b/codex-rs/core/src/unified_exec/process.rs index 69790dbad2..647cf7f3e1 100644 --- a/codex-rs/core/src/unified_exec/process.rs +++ b/codex-rs/core/src/unified_exec/process.rs @@ -24,6 +24,12 @@ use super::UNIFIED_EXEC_OUTPUT_MAX_TOKENS; use super::UnifiedExecError; use super::head_tail_buffer::HeadTailBuffer; +#[cfg(unix)] +pub(crate) use codex_shell_escalation::EscalationSession; +#[cfg(not(unix))] +#[derive(Debug)] +pub(crate) struct EscalationSession; + pub(crate) type OutputBuffer = Arc>; pub(crate) struct OutputHandles { pub(crate) output_buffer: OutputBuffer, @@ -44,6 +50,7 @@ pub(crate) struct UnifiedExecProcess { output_drained: Arc, output_task: JoinHandle<()>, sandbox_type: SandboxType, + _escalation_session: Option, } impl UnifiedExecProcess { @@ -51,6 +58,7 @@ impl UnifiedExecProcess { process_handle: ExecCommandSession, initial_output_rx: tokio::sync::broadcast::Receiver>, sandbox_type: SandboxType, + escalation_session: Option, ) -> Self { let output_buffer = Arc::new(Mutex::new(HeadTailBuffer::default())); let output_notify = Arc::new(Notify::new()); @@ -92,6 +100,7 @@ impl UnifiedExecProcess { output_drained, output_task, sandbox_type, + _escalation_session: escalation_session, } } @@ -196,13 +205,14 @@ impl UnifiedExecProcess { pub(super) async fn from_spawned( spawned: SpawnedPty, sandbox_type: SandboxType, + escalation_session: Option, ) -> Result { let SpawnedPty { session: process_handle, output_rx, mut exit_rx, } = spawned; - let managed = Self::new(process_handle, output_rx, sandbox_type); + let managed = Self::new(process_handle, output_rx, sandbox_type, escalation_session); let exit_ready = matches!(exit_rx.try_recv(), Ok(_) | Err(TryRecvError::Closed)); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 821abc683e..426a7fb2d5 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -47,6 +47,7 @@ use crate::unified_exec::async_watcher::start_streaming_output; use crate::unified_exec::clamp_yield_time; use crate::unified_exec::generate_chunk_id; use crate::unified_exec::head_tail_buffer::HeadTailBuffer; +use crate::unified_exec::process::EscalationSession; use crate::unified_exec::process::OutputBuffer; use crate::unified_exec::process::OutputHandles; use crate::unified_exec::process::UnifiedExecProcess; @@ -528,6 +529,7 @@ impl UnifiedExecProcessManager { &self, env: &ExecRequest, tty: bool, + escalation_session: Option, ) -> Result { let (program, args) = env .command @@ -555,7 +557,15 @@ impl UnifiedExecProcessManager { }; let spawned = spawn_result.map_err(|err| UnifiedExecError::create_process(err.to_string()))?; - UnifiedExecProcess::from_spawned(spawned, env.sandbox).await + #[cfg(unix)] + let escalation_session = { + let mut escalation_session = escalation_session; + if let Some(session) = escalation_session.as_mut() { + session.close_client_socket(); + } + escalation_session + }; + UnifiedExecProcess::from_spawned(spawned, env.sandbox, escalation_session).await } pub(super) async fn open_session_with_sandbox( @@ -569,7 +579,8 @@ impl UnifiedExecProcessManager { Some(context.session.conversation_id), )); let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = UnifiedExecRuntime::new(self); + let mut runtime = + UnifiedExecRuntime::new(self, context.turn.tools_config.unified_exec_backend); let exec_approval_requirement = context .session .services diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 67a8b8e604..56c4ce70d1 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -478,6 +478,7 @@ mod tests { .expect("absolute readable root"), ], }, + network_access: false, }; let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); @@ -503,6 +504,7 @@ mod tests { include_platform_defaults: true, readable_roots: Vec::new(), }, + network_access: false, }; // `ReadOnlyAccess::Restricted` always includes `cwd` as a readable diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2591a8fb1d..55f19255b0 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -626,6 +626,11 @@ pub enum SandboxPolicy { skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" )] access: ReadOnlyAccess, + + /// When set to `true`, outbound network access is allowed. `false` by + /// default. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + network_access: bool, }, /// Indicates the process is already in an external sandbox. Allows full @@ -715,6 +720,7 @@ impl SandboxPolicy { pub fn new_read_only_policy() -> Self { SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, } } @@ -735,7 +741,7 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { .. } => true, - SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(), + SandboxPolicy::ReadOnly { access, .. } => access.has_full_disk_read_access(), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => read_only_access.has_full_disk_read_access(), @@ -755,7 +761,7 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(), - SandboxPolicy::ReadOnly { .. } => false, + SandboxPolicy::ReadOnly { network_access, .. } => *network_access, SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access, } } @@ -766,7 +772,7 @@ impl SandboxPolicy { return false; } match self { - SandboxPolicy::ReadOnly { access } => access.include_platform_defaults(), + SandboxPolicy::ReadOnly { access, .. } => access.include_platform_defaults(), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => read_only_access.include_platform_defaults(), @@ -782,7 +788,7 @@ impl SandboxPolicy { pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { let mut roots = match self { SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(), - SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd), + SandboxPolicy::ReadOnly { access, .. } => access.get_readable_roots_with_cwd(cwd), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => { @@ -3131,6 +3137,18 @@ mod tests { assert!(enabled.has_full_network_access()); } + #[test] + fn read_only_reports_network_access_flags() { + let restricted = SandboxPolicy::new_read_only_policy(); + assert!(!restricted.has_full_network_access()); + + let enabled = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, + }; + assert!(enabled.has_full_network_access()); + } + #[test] fn reject_config_mcp_elicitation_flag_is_field_driven() { assert!( diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs index 1cc42a46db..6d08a78716 100644 --- a/codex-rs/shell-escalation/src/lib.rs +++ b/codex-rs/shell-escalation/src/lib.rs @@ -14,6 +14,8 @@ pub use unix::EscalationPermissions; #[cfg(unix)] pub use unix::EscalationPolicy; #[cfg(unix)] +pub use unix::EscalationSession; +#[cfg(unix)] pub use unix::ExecParams; #[cfg(unix)] pub use unix::ExecResult; diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index bb45c3ed71..578c98616f 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -7,7 +7,9 @@ use std::time::Duration; use anyhow::Context as _; use codex_utils_absolute_path::AbsolutePathBuf; +use socket2::Socket; use tokio::process::Command; +use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; @@ -82,6 +84,29 @@ pub struct PreparedExec { pub arg0: Option, } +#[derive(Debug)] +pub struct EscalationSession { + env: HashMap, + task: JoinHandle>, + client_socket: Option, +} + +impl EscalationSession { + pub fn env(&self) -> &HashMap { + &self.env + } + + pub fn close_client_socket(&mut self) { + self.client_socket.take(); + } +} + +impl Drop for EscalationSession { + fn drop(&mut self) { + self.task.abort(); + } +} + pub struct EscalateServer { bash_path: PathBuf, execve_wrapper: PathBuf, @@ -106,16 +131,42 @@ impl EscalateServer { cancel_rx: CancellationToken, command_executor: Arc, ) -> anyhow::Result { + let session = self.start_session(Arc::clone(&command_executor))?; + let command = vec![ + self.bash_path.to_string_lossy().to_string(), + if params.login == Some(false) { + "-c".to_string() + } else { + "-lc".to_string() + }, + params.command, + ]; + let workdir = AbsolutePathBuf::try_from(params.workdir)?; + let result = command_executor + .run( + command, + workdir.to_path_buf(), + session.env().clone(), + cancel_rx, + ) + .await?; + Ok(result) + } + + pub fn start_session( + &self, + command_executor: Arc, + ) -> anyhow::Result { let (escalate_server, escalate_client) = AsyncDatagramSocket::pair()?; let client_socket = escalate_client.into_inner(); // Only the client endpoint should cross exec into the wrapper process. client_socket.set_cloexec(false)?; - let escalate_task = tokio::spawn(escalate_task( + let task = tokio::spawn(escalate_task( escalate_server, Arc::clone(&self.policy), Arc::clone(&command_executor), )); - let mut env = std::env::vars().collect::>(); + let mut env = HashMap::new(); env.insert( ESCALATE_SOCKET_ENV_VAR.to_string(), client_socket.as_raw_fd().to_string(), @@ -128,22 +179,11 @@ impl EscalateServer { LEGACY_BASH_EXEC_WRAPPER_ENV_VAR.to_string(), self.execve_wrapper.to_string_lossy().to_string(), ); - - let command = vec![ - self.bash_path.to_string_lossy().to_string(), - if params.login == Some(false) { - "-c".to_string() - } else { - "-lc".to_string() - }, - params.command, - ]; - let workdir = AbsolutePathBuf::try_from(params.workdir)?; - let result = command_executor - .run(command, workdir.to_path_buf(), env, cancel_rx) - .await?; - escalate_task.abort(); - Ok(result) + Ok(EscalationSession { + env, + task, + client_socket: Some(client_socket), + }) } } @@ -390,6 +430,37 @@ mod tests { } } + #[tokio::test] + async fn start_session_exposes_wrapper_env_overlay() -> anyhow::Result<()> { + let execve_wrapper = PathBuf::from("/tmp/codex-execve-wrapper"); + let execve_wrapper_str = execve_wrapper.to_string_lossy().to_string(); + let server = EscalateServer::new( + PathBuf::from("/bin/bash"), + execve_wrapper.clone(), + DeterministicEscalationPolicy { + decision: EscalationDecision::run(), + }, + ); + + let mut session = server.start_session(Arc::new(ForwardingShellCommandExecutor))?; + let env = session.env(); + assert_eq!(env.get(EXEC_WRAPPER_ENV_VAR), Some(&execve_wrapper_str)); + assert_eq!( + env.get(LEGACY_BASH_EXEC_WRAPPER_ENV_VAR), + Some(&execve_wrapper_str) + ); + let socket_fd = env + .get(ESCALATE_SOCKET_ENV_VAR) + .expect("session should export shell escalation socket"); + let socket_fd = socket_fd.parse::()?; + assert!(socket_fd >= 0); + assert_ne!(unsafe { libc::fcntl(socket_fd, libc::F_GETFD) }, -1); + session.close_client_socket(); + assert_eq!(unsafe { libc::fcntl(socket_fd, libc::F_GETFD) }, -1); + + Ok(()) + } + #[tokio::test] async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?; diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs index 6de12297a4..13b7146ab4 100644 --- a/codex-rs/shell-escalation/src/unix/mod.rs +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -66,6 +66,7 @@ pub use self::escalate_protocol::EscalateAction; pub use self::escalate_protocol::EscalationDecision; pub use self::escalate_protocol::EscalationExecution; pub use self::escalate_server::EscalateServer; +pub use self::escalate_server::EscalationSession; pub use self::escalate_server::ExecParams; pub use self::escalate_server::ExecResult; pub use self::escalate_server::PreparedExec; diff --git a/codex-rs/shell-escalation/src/unix/stopwatch.rs b/codex-rs/shell-escalation/src/unix/stopwatch.rs index f564434813..19821d8be2 100644 --- a/codex-rs/shell-escalation/src/unix/stopwatch.rs +++ b/codex-rs/shell-escalation/src/unix/stopwatch.rs @@ -9,7 +9,7 @@ use tokio_util::sync::CancellationToken; #[derive(Clone, Debug)] pub struct Stopwatch { - limit: Duration, + limit: Option, inner: Arc>, notify: Arc, } @@ -30,13 +30,27 @@ impl Stopwatch { active_pauses: 0, })), notify: Arc::new(Notify::new()), - limit, + limit: Some(limit), + } + } + + pub fn unlimited() -> Self { + Self { + inner: Arc::new(Mutex::new(StopwatchState { + elapsed: Duration::ZERO, + running_since: Some(Instant::now()), + active_pauses: 0, + })), + notify: Arc::new(Notify::new()), + limit: None, } } pub fn cancellation_token(&self) -> CancellationToken { - let limit = self.limit; let token = CancellationToken::new(); + let Some(limit) = self.limit else { + return token; + }; let cancel = token.clone(); let inner = Arc::clone(&self.inner); let notify = Arc::clone(&self.notify); @@ -208,4 +222,16 @@ mod tests { // Now the stopwatch should resume and hit the limit shortly after. token.cancelled().await; } + + #[tokio::test] + async fn unlimited_stopwatch_never_cancels() { + let stopwatch = Stopwatch::unlimited(); + let token = stopwatch.cancellation_token(); + + assert!( + timeout(Duration::from_millis(30), token.cancelled()) + .await + .is_err() + ); + } } diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index a9ef2cd38a..548be75769 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -641,6 +641,9 @@ fn format_additional_permissions_rule( additional_permissions: &PermissionProfile, ) -> Option { let mut parts = Vec::new(); + if matches!(additional_permissions.network, Some(true)) { + parts.push("network".to_string()); + } if let Some(file_system) = additional_permissions.file_system.as_ref() { if let Some(read) = file_system.read.as_ref() { let reads = read @@ -1074,6 +1077,7 @@ mod tests { available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, additional_permissions: Some(PermissionProfile { + network: Some(true), file_system: Some(FileSystemPermissions { read: Some(vec![absolute_path("/tmp/readme.txt")]), write: Some(vec![absolute_path("/tmp/out.txt")]), @@ -1100,6 +1104,10 @@ mod tests { .any(|line| line.contains("Permission rule:")), "expected permission-rule line, got {rendered:?}" ); + assert!( + rendered.iter().any(|line| line.contains("network;")), + "expected network permission text, got {rendered:?}" + ); } #[test] @@ -1115,6 +1123,7 @@ mod tests { available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, additional_permissions: Some(PermissionProfile { + network: Some(true), file_system: Some(FileSystemPermissions { read: Some(vec![absolute_path("/tmp/readme.txt")]), write: Some(vec![absolute_path("/tmp/out.txt")]), diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_prompt.snap index 5b04b78b12..989f80f572 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_prompt.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_additional_permissions_prompt.snap @@ -7,7 +7,7 @@ expression: "render_overlay_lines(&view, 120)" Reason: need filesystem access - Permission rule: read `/tmp/readme.txt`; write `/tmp/out.txt` + Permission rule: network; read `/tmp/readme.txt`; write `/tmp/out.txt` $ cat /tmp/readme.txt diff --git a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs index a5c9b31fdd..a65d6b2ce9 100644 --- a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs +++ b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs @@ -4,7 +4,13 @@ use codex_protocol::protocol::SandboxPolicy; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), + SandboxPolicy::ReadOnly { network_access, .. } => { + let mut summary = "read-only".to_string(); + if *network_access { + summary.push_str(" (network access enabled)"); + } + summary + } SandboxPolicy::ExternalSandbox { network_access } => { let mut summary = "external-sandbox".to_string(); if matches!(network_access, NetworkAccess::Enabled) { @@ -66,6 +72,15 @@ mod tests { assert_eq!(summary, "external-sandbox (network access enabled)"); } + #[test] + fn summarizes_read_only_with_enabled_network() { + let summary = summarize_sandbox_policy(&SandboxPolicy::ReadOnly { + access: Default::default(), + network_access: true, + }); + assert_eq!(summary, "read-only (network access enabled)"); + } + #[test] fn workspace_write_summary_still_includes_network_access() { let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };