From 3b52dd7b7d94d5fbd23e6315166e410fef4d308c Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Sat, 21 Mar 2026 15:19:11 -0700 Subject: [PATCH] Add request_permissions profile persistence core support Co-authored-by: Codex --- codex-rs/core/src/codex.rs | 35 +++- codex-rs/core/src/codex_delegate.rs | 1 + codex-rs/core/src/codex_delegate_tests.rs | 2 + codex-rs/core/src/lib.rs | 1 + .../src/permission_profile_persistence.rs | 179 ++++++++++++++++++ .../core/tests/suite/request_permissions.rs | 7 + .../tests/suite/request_permissions_tool.rs | 2 + codex-rs/protocol/src/protocol.rs | 9 + codex-rs/protocol/src/request_permissions.rs | 8 + 9 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 codex-rs/core/src/permission_profile_persistence.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cbaabe6b84..ae74c2d9f9 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -33,6 +33,7 @@ use crate::models_manager::manager::ModelsManager; use crate::models_manager::manager::RefreshStrategy; use crate::parse_command::parse_command; use crate::parse_turn_item; +use crate::permission_profile_persistence::persistence_target_for_permissions; use crate::realtime_conversation::RealtimeConversationManager; use crate::realtime_conversation::handle_audio as handle_realtime_conversation_audio; use crate::realtime_conversation::handle_close as handle_realtime_conversation_close; @@ -2998,7 +2999,11 @@ impl Session { call_id, turn_id: turn_context.sub_id.clone(), reason: args.reason, - permissions: args.permissions, + permissions: args.permissions.clone(), + permissions_profile_persistence: persistence_target_for_permissions( + turn_context.config.as_ref(), + &args.permissions.into(), + ), }); self.send_event(turn_context, event).await; rx_response.await.ok() @@ -4253,8 +4258,18 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv handlers::request_user_input_response(&sess, id, response).await; false } - Op::RequestPermissionsResponse { id, response } => { - handlers::request_permissions_response(&sess, id, response).await; + Op::RequestPermissionsResponse { + id, + response, + persist_permissions, + } => { + handlers::request_permissions_response( + &sess, + id, + response, + persist_permissions, + ) + .await; false } Op::DynamicToolResponse { id, response } => { @@ -4400,6 +4415,7 @@ mod handlers { use crate::codex::spawn_review_thread; use crate::config::Config; + use crate::permission_profile_persistence::persist_permissions_for_profile; use crate::mcp::auth::compute_auth_statuses; use crate::mcp::collect_mcp_snapshot_from_manager; @@ -4420,6 +4436,7 @@ mod handlers { use codex_protocol::protocol::ListSkillsResponseEvent; use codex_protocol::protocol::McpServerRefreshConfig; use codex_protocol::protocol::Op; + use codex_protocol::protocol::PersistPermissionProfileAction; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::RolloutItem; @@ -4681,7 +4698,19 @@ mod handlers { sess: &Arc, id: String, response: RequestPermissionsResponse, + persist_permissions: Option, ) { + if let Some(action) = persist_permissions.as_ref() + && let Err(err) = persist_permissions_for_profile(sess.as_ref(), action).await + { + let message = format!("Failed to update permissions profile: {err}"); + tracing::warn!("{message}"); + sess.send_event_raw(Event { + id: id.clone(), + msg: EventMsg::Warning(WarningEvent { message }), + }) + .await; + } sess.notify_request_permissions_response(&id, response) .await; } diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index e560cd9c7f..279f60f5fc 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -764,6 +764,7 @@ async fn handle_request_permissions( .submit(Op::RequestPermissionsResponse { id: call_id, response, + persist_permissions: None, }) .await; } diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 8201424d8e..dda6da513a 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -200,6 +200,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { }), ..RequestPermissionProfile::default() }, + permissions_profile_persistence: None, }, &cancel_token, ) @@ -234,6 +235,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { Op::RequestPermissionsResponse { id: call_id, response: expected_response, + persist_permissions: None, } ); } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 29436a0d7f..ae4e450052 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -54,6 +54,7 @@ mod network_policy_decision; pub mod network_proxy_loader; mod original_image_detail; mod packages; +mod permission_profile_persistence; pub use mcp_connection_manager::MCP_SANDBOX_STATE_CAPABILITY; pub use mcp_connection_manager::MCP_SANDBOX_STATE_METHOD; pub use mcp_connection_manager::SandboxState; diff --git a/codex-rs/core/src/permission_profile_persistence.rs b/codex-rs/core/src/permission_profile_persistence.rs new file mode 100644 index 0000000000..2e10fdde3f --- /dev/null +++ b/codex-rs/core/src/permission_profile_persistence.rs @@ -0,0 +1,179 @@ +use std::collections::BTreeMap; +use std::io; + +use toml_edit::value; + +use crate::codex::Session; +use crate::config::Config; +use crate::config::deserialize_config_toml_with_base; +use crate::config::edit::ConfigEdit; +use crate::config::edit::ConfigEditsBuilder; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::request_permissions::PermissionProfilePersistence; + +pub(crate) fn persistence_target_for_permissions( + config: &Config, + permissions: &PermissionProfile, +) -> Option { + if !is_supported_filesystem_only_request(permissions) { + return None; + } + + let user_layer = config.config_layer_stack.get_user_layer()?; + let user_config = + deserialize_config_toml_with_base(user_layer.config.clone(), &config.codex_home).ok()?; + let profile_name = user_config.default_permissions?; + let permissions = user_config.permissions?; + permissions + .entries + .contains_key(profile_name.as_str()) + .then_some(PermissionProfilePersistence { profile_name }) +} + +pub(crate) async fn persist_permissions_for_profile( + sess: &Session, + action: &codex_protocol::protocol::PersistPermissionProfileAction, +) -> io::Result<()> { + let codex_home = sess.codex_home().await; + + let edits = filesystem_permission_edits( + action.profile_name.as_str(), + action.permissions.file_system.as_ref(), + ); + if edits.is_empty() { + return Ok(()); + } + + ConfigEditsBuilder::new(&codex_home) + .with_edits(edits) + .apply() + .await + .map_err(|err| io::Error::other(format!("failed to persist permission profile: {err}")))?; + sess.reload_user_config_layer().await; + Ok(()) +} + +fn is_supported_filesystem_only_request(permissions: &PermissionProfile) -> bool { + let Some(file_system) = permissions.file_system.as_ref() else { + return false; + }; + + if file_system.is_empty() { + return false; + } + + if permissions + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + { + return false; + } + + permissions.macos.is_none() +} + +fn filesystem_permission_edits( + profile_name: &str, + file_system: Option<&FileSystemPermissions>, +) -> Vec { + let Some(file_system) = file_system else { + return Vec::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 + .into_iter() + .map(|(path, access)| ConfigEdit::SetPath { + segments: vec![ + "permissions".to_string(), + profile_name.to_string(), + "filesystem".to_string(), + path, + ], + value: value(access.to_string()), + }) + .collect() +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use super::*; + use codex_utils_absolute_path::AbsolutePathBuf; + + fn absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(path).expect("absolute path") + } + + #[test] + fn filesystem_permission_edits_upgrade_write_access() { + let edits = filesystem_permission_edits( + "workspace", + Some(&FileSystemPermissions { + read: Some(vec![ + absolute_path("/tmp/read"), + absolute_path("/tmp/write"), + ]), + write: Some(vec![absolute_path("/tmp/write")]), + }), + ); + + assert_eq!(edits.len(), 2); + match &edits[0] { + ConfigEdit::SetPath { segments, value } => { + assert_eq!( + segments, + &[ + "permissions".to_string(), + "workspace".to_string(), + "filesystem".to_string(), + "/tmp/read".to_string(), + ] + ); + assert_eq!( + value.as_value().and_then(toml_edit::Value::as_str), + Some("read") + ); + } + other => panic!("unexpected edit: {other:?}"), + } + match &edits[1] { + ConfigEdit::SetPath { segments, value } => { + assert_eq!( + segments, + &[ + "permissions".to_string(), + "workspace".to_string(), + "filesystem".to_string(), + "/tmp/write".to_string(), + ] + ); + assert_eq!( + value.as_value().and_then(toml_edit::Value::as_str), + Some("write") + ); + } + other => panic!("unexpected edit: {other:?}"), + } + } +} diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index b1aaac65b4..0f730fd8c4 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -1087,6 +1087,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul permissions: normalized_requested_permissions.clone(), scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -1201,6 +1202,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req permissions: normalized_requested_permissions, scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -1314,6 +1316,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu permissions: normalized_requested_permissions.clone(), scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -1423,6 +1426,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls_without_i permissions: normalized_requested_permissions.clone(), scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -1569,6 +1573,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() permissions: granted_permissions.clone(), scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -1687,6 +1692,7 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { permissions: normalized_requested_permissions, scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; wait_for_completion(&test).await; @@ -1804,6 +1810,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> { permissions: normalized_requested_permissions, scope: PermissionGrantScope::Session, }, + persist_permissions: None, }) .await?; wait_for_completion(&test).await; diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index a01d6e0ab7..9053b4c750 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -261,6 +261,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s permissions: normalized_requested_permissions, scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; @@ -380,6 +381,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_apply_patch_wi permissions: normalized_requested_permissions, scope: PermissionGrantScope::Turn, }, + persist_permissions: None, }) .await?; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4f7f2616f7..66a13a5d0f 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -403,6 +403,9 @@ pub enum Op { id: String, /// User-granted permissions. response: RequestPermissionsResponse, + /// Optional permission-profile mutation to persist alongside the grant. + #[serde(default, skip_serializing_if = "Option::is_none")] + persist_permissions: Option, }, /// Resolve a dynamic tool call request. @@ -3211,6 +3214,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")] diff --git a/codex-rs/protocol/src/request_permissions.rs b/codex-rs/protocol/src/request_permissions.rs index db5396c5e4..991ad6e427 100644 --- a/codex-rs/protocol/src/request_permissions.rs +++ b/codex-rs/protocol/src/request_permissions.rs @@ -14,6 +14,11 @@ pub enum PermissionGrantScope { Session, } +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +pub struct PermissionProfilePersistence { + pub profile_name: String, +} + #[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] #[serde(deny_unknown_fields)] pub struct RequestPermissionProfile { @@ -71,4 +76,7 @@ pub struct RequestPermissionsEvent { #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, pub permissions: RequestPermissionProfile, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub permissions_profile_persistence: Option, }