Merge 8898ed33b0 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-03-03 20:53:39 -08:00
committed by GitHub
35 changed files with 895 additions and 70 deletions
@@ -1637,6 +1637,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -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"
@@ -11972,6 +11972,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -9752,6 +9752,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -89,6 +89,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -653,6 +653,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -653,6 +653,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -653,6 +653,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -214,6 +214,10 @@
"type": "fullAccess"
}
},
"networkAccess": {
"default": false,
"type": "boolean"
},
"type": {
"enum": [
"readOnly"
@@ -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.
*/
@@ -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<AbsolutePathBuf>, 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<AbsolutePathBuf>, readOnlyAccess: ReadOnlyAccess, networkAccess: boolean, excludeTmpdirEnvVar: boolean, excludeSlashTmp: boolean, };
+19 -10
View File
@@ -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<codex_protocol::protocol::SandboxPolicy> 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,
}
);
}
@@ -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,
)
+7
View File
@@ -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"
+73 -3
View File
@@ -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<String>,
}
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<String> {
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;
+28 -9
View File
@@ -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,
},
)]);
+222
View File
@@ -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<String, AgentRoleToml>,
@@ -1396,6 +1398,8 @@ pub struct AgentRoleConfig {
pub description: Option<String>,
/// Path to a role-specific config layer.
pub config_file: Option<PathBuf>,
/// Candidate nicknames for agents spawned with this role.
pub nickname_candidates: Option<Vec<String>>,
}
#[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<AbsolutePathBuf>,
/// Candidate nicknames for agents spawned with this role.
pub nickname_candidates: Option<Vec<String>>,
}
impl From<ToolsToml> 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<Option<Vec<String>>> {
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::<Vec<_>>()),
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::<Vec<_>>()),
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 <One>".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(())
}
+90 -6
View File
@@ -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,
}
);
}
}
+13 -2
View File
@@ -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,
+1 -1
View File
@@ -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;
@@ -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<Option<PreparedUnifiedExecZshFork>, 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<RwLock<Policy>>,
session: Arc<crate::codex::Session>,
@@ -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<ParsedShellCommand, ToolEr
// Commands reaching zsh-fork can be wrapped by environment/sandbox helpers, so
// we search for the first `-c`/`-lc` triple anywhere in the argv rather
// than assuming it is the first positional form.
if let Some((script, login)) = command.windows(3).find_map(|parts| match parts {
[_, flag, script] if flag == "-c" => 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(
@@ -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,
}
@@ -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<UnifiedExecRequest, UnifiedExecProcess> 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<UnifiedExecRequest, UnifiedExecProcess> 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, .. } => {
+18
View File
@@ -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<ApplyPatchToolType>,
pub web_search_mode: Option<WebSearchMode>,
@@ -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]
+11 -1
View File
@@ -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<Mutex<HeadTailBuffer>>;
pub(crate) struct OutputHandles {
pub(crate) output_buffer: OutputBuffer,
@@ -44,6 +50,7 @@ pub(crate) struct UnifiedExecProcess {
output_drained: Arc<Notify>,
output_task: JoinHandle<()>,
sandbox_type: SandboxType,
_escalation_session: Option<EscalationSession>,
}
impl UnifiedExecProcess {
@@ -51,6 +58,7 @@ impl UnifiedExecProcess {
process_handle: ExecCommandSession,
initial_output_rx: tokio::sync::broadcast::Receiver<Vec<u8>>,
sandbox_type: SandboxType,
escalation_session: Option<EscalationSession>,
) -> 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<EscalationSession>,
) -> Result<Self, UnifiedExecError> {
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));
@@ -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<EscalationSession>,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
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
+2
View File
@@ -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
+22 -4
View File
@@ -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<AbsolutePathBuf> {
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!(
+2
View File
@@ -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;
@@ -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<String>,
}
#[derive(Debug)]
pub struct EscalationSession {
env: HashMap<String, String>,
task: JoinHandle<anyhow::Result<()>>,
client_socket: Option<Socket>,
}
impl EscalationSession {
pub fn env(&self) -> &HashMap<String, String> {
&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<dyn ShellCommandExecutor>,
) -> anyhow::Result<ExecResult> {
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<dyn ShellCommandExecutor>,
) -> anyhow::Result<EscalationSession> {
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::<HashMap<String, String>>();
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::<i32>()?;
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()?;
@@ -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;
@@ -9,7 +9,7 @@ use tokio_util::sync::CancellationToken;
#[derive(Clone, Debug)]
pub struct Stopwatch {
limit: Duration,
limit: Option<Duration>,
inner: Arc<Mutex<StopwatchState>>,
notify: Arc<Notify>,
}
@@ -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()
);
}
}
@@ -641,6 +641,9 @@ fn format_additional_permissions_rule(
additional_permissions: &PermissionProfile,
) -> Option<String> {
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")]),
@@ -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
@@ -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" };