diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 0f1189b22d..8dca6ddf03 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -7,7 +7,10 @@ use codex_features::FEATURES; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ServiceTier; use codex_protocol::config_types::TrustLevel; +use codex_protocol::models::FileSystemPermissions; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::protocol::PersistPermissionProfileAction; use std::collections::BTreeMap; use std::collections::HashMap; use std::path::Path; @@ -59,6 +62,8 @@ pub enum ConfigEdit { segments: Vec, value: TomlItem, }, + /// Merge permissions into a named profile. + MergePermissionProfile(PersistPermissionProfileAction), /// Remove the value stored at the exact dotted path. ClearPath { segments: Vec }, } @@ -426,6 +431,7 @@ impl ConfigDocument { Ok(self.set_skill_config(SkillConfigSelector::Name(name.clone()), *enabled)) } ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())), + ConfigEdit::MergePermissionProfile(action) => Ok(self.merge_permission_profile(action)), ConfigEdit::ClearPath { segments } => Ok(self.clear_owned(segments)), ConfigEdit::SetProjectTrustLevel { path, level } => { // Delegate to the existing, tested logic in config.rs to @@ -461,6 +467,173 @@ impl ConfigDocument { self.remove(segments) } + fn merge_permission_profile(&mut self, action: &PersistPermissionProfileAction) -> bool { + let mut mutated = false; + + for (path, access) in filesystem_path_access(action.permissions.file_system.as_ref()) { + if access == FileSystemAccessMode::Read + && self.has_writable_filesystem_ancestor(&action.profile_name, &path) + { + continue; + } + + let segments = vec![ + "permissions".to_string(), + action.profile_name.clone(), + "filesystem".to_string(), + path.clone(), + ]; + let merged_access = match self.get_item(&segments).and_then(TomlItem::as_str) { + Some("write") => FileSystemAccessMode::Write, + Some("read") => access, + _ => access, + }; + mutated |= self.insert(&segments, value(merged_access.to_string())); + + if merged_access == FileSystemAccessMode::Write { + mutated |= + self.remove_descendant_filesystem_reads(&action.profile_name, path.as_str()); + } + } + + if let Some(enabled) = action + .permissions + .network + .as_ref() + .and_then(|network| network.enabled) + { + let segments = vec![ + "permissions".to_string(), + action.profile_name.clone(), + "network".to_string(), + "enabled".to_string(), + ]; + let merged_enabled = self + .get_item(&segments) + .and_then(TomlItem::as_bool) + .unwrap_or(false) + || enabled; + mutated |= self.insert(&segments, value(merged_enabled)); + } + + mutated + } + + fn has_writable_filesystem_ancestor(&mut self, profile_name: &str, path: &str) -> bool { + let filesystem_segments = vec![ + "permissions".to_string(), + profile_name.to_string(), + "filesystem".to_string(), + ]; + let Some(filesystem) = self.descend(&filesystem_segments, TraversalMode::Existing) else { + return false; + }; + + let path = Path::new(path); + for (key, item) in filesystem.iter() { + if matches!(item.as_str(), Some("write")) { + let candidate = Path::new(key); + if path.starts_with(candidate) && path != candidate { + return true; + } + continue; + } + + let Some(scoped_entries) = item.as_table() else { + continue; + }; + let base = Path::new(key); + if !base.is_absolute() { + continue; + } + + for (subpath, access) in scoped_entries.iter() { + if !matches!(access.as_str(), Some("write")) { + continue; + } + + let candidate = if subpath == "." { + base.to_path_buf() + } else { + base.join(subpath) + }; + if path.starts_with(&candidate) && path != candidate { + return true; + } + } + } + false + } + + fn remove_descendant_filesystem_reads(&mut self, profile_name: &str, path: &str) -> bool { + let filesystem_segments = vec![ + "permissions".to_string(), + profile_name.to_string(), + "filesystem".to_string(), + ]; + let Some(filesystem) = self.descend(&filesystem_segments, TraversalMode::Existing) else { + return false; + }; + + let path = Path::new(path); + let mut descendants_to_remove = Vec::new(); + let mut scoped_parents_to_prune = Vec::new(); + for (key, item) in filesystem.iter() { + if matches!(item.as_str(), Some("read")) { + let candidate = Path::new(key); + if candidate.starts_with(path) && candidate != path { + descendants_to_remove.push(vec![key.to_string()]); + } + continue; + } + + let Some(scoped_entries) = item.as_table() else { + continue; + }; + let base = Path::new(key); + if !base.is_absolute() { + continue; + } + + for (subpath, access) in scoped_entries.iter() { + if !matches!(access.as_str(), Some("read")) { + continue; + } + + let candidate = if subpath == "." { + base.to_path_buf() + } else { + base.join(subpath) + }; + if candidate.starts_with(path) && candidate != path { + descendants_to_remove.push(vec![key.to_string(), subpath.to_string()]); + scoped_parents_to_prune.push(key.to_string()); + } + } + } + + let mut mutated = false; + for descendant in descendants_to_remove { + let mut segments = filesystem_segments.clone(); + segments.extend(descendant); + mutated |= self.remove(&segments); + } + scoped_parents_to_prune.sort(); + scoped_parents_to_prune.dedup(); + for parent in scoped_parents_to_prune { + let mut segments = filesystem_segments.clone(); + segments.push(parent); + if self + .get_item(&segments) + .and_then(TomlItem::as_table) + .is_some_and(TomlTable::is_empty) + { + mutated |= self.remove(&segments); + } + } + mutated + } + fn replace_mcp_servers(&mut self, servers: &BTreeMap) -> bool { if servers.is_empty() { return self.clear(Scope::Global, &["mcp_servers"]); @@ -674,6 +847,12 @@ impl ConfigDocument { parent.remove(last).is_some() } + fn get_item(&mut self, segments: &[String]) -> Option<&TomlItem> { + let (last, parents) = segments.split_last()?; + let parent = self.descend(parents, TraversalMode::Existing)?; + parent.get(last.as_str()) + } + fn descend(&mut self, segments: &[String], mode: TraversalMode) -> Option<&mut TomlTable> { let mut current = self.doc.as_table_mut(); @@ -739,6 +918,32 @@ fn normalize_skill_config_path(path: &Path) -> String { .to_string() } +fn filesystem_path_access( + file_system: Option<&FileSystemPermissions>, +) -> BTreeMap { + let Some(file_system) = file_system else { + return BTreeMap::new(); + }; + + let mut path_access = BTreeMap::new(); + + if let Some(read_roots) = file_system.read.as_ref() { + for path in read_roots { + path_access + .entry(path.display().to_string()) + .or_insert(FileSystemAccessMode::Read); + } + } + + if let Some(write_roots) = file_system.write.as_ref() { + for path in write_roots { + path_access.insert(path.display().to_string(), FileSystemAccessMode::Write); + } + } + + path_access +} + fn skill_config_selector_from_table(table: &TomlTable) -> Option { let path = table .get("path") @@ -1059,6 +1264,11 @@ impl ConfigEditsBuilder { self } + pub fn merge_permission_profile(mut self, action: PersistPermissionProfileAction) -> Self { + self.edits.push(ConfigEdit::MergePermissionProfile(action)); + self + } + /// Apply edits on a blocking thread. pub fn apply_blocking(self) -> anyhow::Result<()> { apply_blocking(&self.codex_home, self.profile.as_deref(), &self.edits) diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index af1251b34f..70a238520c 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -2,7 +2,11 @@ use super::*; use codex_config::types::AppToolApproval; use codex_config::types::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::NetworkPermissions; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::protocol::PersistPermissionProfileAction; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; #[cfg(unix)] use std::os::unix::fs::symlink; @@ -48,6 +52,236 @@ fn builder_with_edits_applies_custom_paths() { assert_eq!(contents, "enabled = true\n"); } +fn absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(path).expect("absolute path") +} + +fn toml_literal_key(key: &str) -> String { + assert!( + !key.contains('\''), + "test keys must not contain single quotes" + ); + format!("'{key}'") +} + +fn path_key(path: &AbsolutePathBuf) -> String { + toml_literal_key(&path.display().to_string()) +} + +fn filesystem_entry_config(path: &AbsolutePathBuf, access: &str) -> String { + format!( + "[permissions.workspace.filesystem]\n{} = \"{access}\"\n", + path_key(path) + ) +} + +fn scoped_filesystem_entry_config( + base: &AbsolutePathBuf, + relative_path: &str, + access: &str, +) -> String { + format!( + "[permissions.workspace.filesystem.{}]\n{} = \"{access}\"\n", + path_key(base), + toml_literal_key(relative_path) + ) +} + +fn assert_toml_eq(contents: &str, expected: &str) { + assert_eq!( + toml::from_str::(contents).expect("parse config"), + toml::from_str::(expected).expect("parse expected config"), + ); +} + +#[test] +fn merge_permission_profile_writes_filesystem_and_network_entries() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let read_path = absolute_path("/tmp/read"); + let write_path = absolute_path("/tmp/write"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: Some(FileSystemPermissions { + read: Some(vec![read_path.clone(), write_path.clone()]), + write: Some(vec![write_path.clone()]), + }), + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let mut filesystem = toml::map::Map::new(); + filesystem.insert( + read_path.display().to_string(), + TomlValue::String("read".to_string()), + ); + filesystem.insert( + write_path.display().to_string(), + TomlValue::String("write".to_string()), + ); + + let mut workspace = toml::map::Map::new(); + workspace.insert("filesystem".to_string(), TomlValue::Table(filesystem)); + let mut network = toml::map::Map::new(); + network.insert("enabled".to_string(), TomlValue::Boolean(true)); + workspace.insert("network".to_string(), TomlValue::Table(network)); + + let mut permissions = toml::map::Map::new(); + permissions.insert("workspace".to_string(), TomlValue::Table(workspace)); + + let mut expected = toml::map::Map::new(); + expected.insert("permissions".to_string(), TomlValue::Table(permissions)); + + assert_eq!( + toml::from_str::(&contents).expect("parse config"), + TomlValue::Table(expected) + ); +} + +#[test] +fn merge_permission_profile_preserves_existing_write_access() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let project_path = absolute_path("/tmp/project"); + let seed_config = filesystem_entry_config(&project_path, "write"); + std::fs::write(codex_home.join(CONFIG_TOML_FILE), seed_config.as_bytes()).expect("seed config"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![project_path]), + ..Default::default() + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, seed_config); +} + +#[test] +fn merge_permission_profile_skips_child_read_under_existing_write_parent() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let project_path = absolute_path("/tmp/project"); + let child_path = absolute_path("/tmp/project/src"); + let seed_config = filesystem_entry_config(&project_path, "write"); + std::fs::write(codex_home.join(CONFIG_TOML_FILE), seed_config.as_bytes()).expect("seed config"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![child_path]), + ..Default::default() + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, seed_config); +} + +#[test] +fn merge_permission_profile_skips_child_read_under_existing_scoped_write_parent() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let base_path = absolute_path("/tmp"); + let child_path = absolute_path("/tmp/project/src"); + let seed_config = scoped_filesystem_entry_config(&base_path, "project", "write"); + std::fs::write(codex_home.join(CONFIG_TOML_FILE), seed_config.as_bytes()).expect("seed config"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![child_path]), + ..Default::default() + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, seed_config); +} + +#[test] +fn merge_permission_profile_removes_existing_child_read_under_new_write_parent() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let project_path = absolute_path("/tmp/project"); + let child_path = absolute_path("/tmp/project/src"); + let seed_config = filesystem_entry_config(&child_path, "read"); + std::fs::write(codex_home.join(CONFIG_TOML_FILE), seed_config.as_bytes()).expect("seed config"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + write: Some(vec![project_path.clone()]), + ..Default::default() + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = filesystem_entry_config(&project_path, "write"); + assert_toml_eq(&contents, &expected); +} + +#[test] +fn merge_permission_profile_removes_existing_scoped_child_read_under_new_write_parent() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + let base_path = absolute_path("/tmp"); + let project_path = absolute_path("/tmp/project"); + let seed_config = scoped_filesystem_entry_config(&base_path, "project/src", "read"); + std::fs::write(codex_home.join(CONFIG_TOML_FILE), seed_config.as_bytes()).expect("seed config"); + + ConfigEditsBuilder::new(codex_home) + .merge_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + write: Some(vec![project_path.clone()]), + ..Default::default() + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = filesystem_entry_config(&project_path, "write"); + assert_toml_eq(&contents, &expected); +} + #[test] fn set_model_availability_nux_count_writes_shown_count() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2c2c9cabec..d3e110326c 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3519,6 +3519,12 @@ impl ReviewDecision { } } +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +pub struct PersistPermissionProfileAction { + pub profile_name: String, + pub permissions: crate::models::PermissionProfile, +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")]