feat(config) Add ConfigEdit permission profile merge

Co-authored-by: Codex <noreply@openai.com>
2026-10-24 18:18:41 +00:00 · 2026-04-10 19:10:17 -07:00
parent 58933237cd
commit b672a1ee94
3 changed files with 450 additions and 0 deletions
@@ -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<String>,
        value: TomlItem,
    },
+    /// Merge permissions into a named profile.
+    MergePermissionProfile(PersistPermissionProfileAction),
    /// Remove the value stored at the exact dotted path.
    ClearPath { segments: Vec<String> },
 }
@@ -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<String, McpServerConfig>) -> 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<String, FileSystemAccessMode> {
+    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<SkillConfigSelector> {
    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)
@@ -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::<TomlValue>(contents).expect("parse config"),
+        toml::from_str::<TomlValue>(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::<TomlValue>(&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");
@@ -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")]