diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 57017833a6..4ad1417b0a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -993,6 +993,7 @@ impl From for CommandExecutionApprovalDecision { execpolicy_amendment: proposed_execpolicy_amendment.into(), }, CoreReviewDecision::ApprovedForSession => Self::AcceptForSession, + CoreReviewDecision::ApprovedPersistToProfile => Self::Accept, CoreReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => Self::ApplyNetworkPolicyAmendment { diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 34640a50cf..43108f8703 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -888,6 +888,7 @@ pub(crate) async fn apply_bespoke_event_handling( .submit(Op::RequestPermissionsResponse { id: request.call_id, response: empty, + persist_permissions: None, }) .await { @@ -2464,6 +2465,7 @@ async fn on_request_permissions_response( .submit(Op::RequestPermissionsResponse { id: call_id, response, + persist_permissions: None, }) .await { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cbaabe6b84..595491d119 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -25,6 +25,7 @@ use crate::compact::run_inline_auto_compact_task; use crate::compact::should_use_remote_compact_task; use crate::compact_remote::run_inline_remote_auto_compact_task; use crate::config::ManagedFeatures; +use crate::config::persistence_target_for_permissions; use crate::connectors; use crate::exec_policy::ExecPolicyManager; #[cfg(test)] @@ -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::config::edit::ConfigEditsBuilder; 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,26 @@ mod handlers { sess: &Arc, id: String, response: RequestPermissionsResponse, + persist_permissions: Option, ) { + if let Some(action) = persist_permissions { + let codex_home = sess.codex_home().await; + if let Err(err) = ConfigEditsBuilder::new(&codex_home) + .persist_permission_profile(action) + .apply() + .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; + } else { + sess.reload_user_config_layer().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..1864318809 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -700,6 +700,7 @@ async fn maybe_auto_review_mcp_request_user_input( .map(|option| option.label.clone()) .unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()), ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(), ReviewDecision::Denied | ReviewDecision::Abort => { @@ -764,6 +765,9 @@ async fn handle_request_permissions( .submit(Op::RequestPermissionsResponse { id: call_id, response, + // TODO: Thread the user's persist_permissions choice back through + // parent_session.request_permissions instead of dropping it here. + 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/config/edit.rs b/codex-rs/core/src/config/edit.rs index 2865ace4b2..8378907474 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -8,7 +8,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; @@ -56,6 +59,8 @@ pub enum ConfigEdit { segments: Vec, value: TomlItem, }, + /// Persist filesystem-only permissions into a named profile. + PersistPermissionProfile(PersistPermissionProfileAction), /// Remove the value stored at the exact dotted path. ClearPath { segments: Vec }, } @@ -390,6 +395,9 @@ impl ConfigDocument { Ok(self.set_skill_config(path.as_path(), *enabled)) } ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())), + ConfigEdit::PersistPermissionProfile(action) => { + Ok(self.persist_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 @@ -425,6 +433,23 @@ impl ConfigDocument { self.remove(segments) } + fn persist_permission_profile(&mut self, action: &PersistPermissionProfileAction) -> bool { + filesystem_path_access(action.permissions.file_system.as_ref()) + .into_iter() + .fold(false, |mut mutated, (path, access)| { + mutated |= self.insert( + &[ + "permissions".to_string(), + action.profile_name.clone(), + "filesystem".to_string(), + path, + ], + value(access.to_string()), + ); + mutated + }) + } + fn replace_mcp_servers(&mut self, servers: &BTreeMap) -> bool { if servers.is_empty() { return self.clear(Scope::Global, &["mcp_servers"]); @@ -699,6 +724,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 +} + /// Persist edits using a blocking strategy. pub fn apply_blocking( codex_home: &Path, @@ -975,6 +1026,12 @@ impl ConfigEditsBuilder { self } + pub fn persist_permission_profile(mut self, action: PersistPermissionProfileAction) -> Self { + self.edits + .push(ConfigEdit::PersistPermissionProfile(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 5a31d84dd0..f1893a9c90 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -1,6 +1,9 @@ use super::*; use crate::config::types::McpServerTransportConfig; +use codex_protocol::models::FileSystemPermissions; 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; @@ -46,6 +49,40 @@ 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") +} + +#[test] +fn persist_permission_profile_writes_filesystem_entries() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + ConfigEditsBuilder::new(codex_home) + .persist_permission_profile(PersistPermissionProfileAction { + profile_name: "workspace".to_string(), + permissions: codex_protocol::models::PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![ + absolute_path("/tmp/read"), + absolute_path("/tmp/write"), + ]), + write: Some(vec![absolute_path("/tmp/write")]), + }), + ..Default::default() + }, + }) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[permissions.workspace.filesystem] +"/tmp/read" = "read" +"/tmp/write" = "write" +"#; + assert_eq!(contents, expected); +} + #[test] fn set_model_availability_nux_count_writes_shown_count() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 17abe55c46..48f0e86545 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -128,6 +128,7 @@ pub use permissions::FilesystemPermissionsToml; pub use permissions::NetworkToml; pub use permissions::PermissionProfileToml; pub use permissions::PermissionsToml; +pub(crate) use permissions::persistence_target_for_permissions; pub(crate) use permissions::resolve_permission_profile; pub use service::ConfigService; pub use service::ConfigServiceError; diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 759c269b76..43d5b42812 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -7,17 +7,22 @@ use std::path::PathBuf; use codex_network_proxy::NetworkMode; use codex_network_proxy::NetworkProxyConfig; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::request_permissions::PermissionProfilePersistence; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; +use crate::config::Config; +use crate::config::deserialize_config_toml_with_base; + #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] pub struct PermissionsToml { #[serde(flatten)] @@ -144,6 +149,26 @@ pub(crate) fn network_proxy_config_from_profile_network( ) } +pub(crate) fn persistence_target_for_permissions( + config: &Config, + permissions: &PermissionProfile, +) -> Option { + if !is_supported_filesystem_only_request(permissions) { + return None; + } + + // TODO: honor inherited default permission profiles instead of only the raw user layer. + 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) fn resolve_permission_profile<'a>( permissions: &'a PermissionsToml, profile_name: &str, @@ -232,6 +257,27 @@ fn compile_network_sandbox_policy(network: Option<&NetworkToml>) -> NetworkSandb } } +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 compile_filesystem_permission( path: &str, permission: &FilesystemPermissionToml, diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index f82cc73bb5..c68c1d6a43 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -747,6 +747,7 @@ pub(crate) fn build_guardian_mcp_tool_review_request( fn mcp_tool_approval_decision_from_guardian(decision: ReviewDecision) -> McpToolApprovalDecision { match decision { ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => McpToolApprovalDecision::Accept, ReviewDecision::ApprovedForSession => McpToolApprovalDecision::AcceptForSession, diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 277ff2b241..18ed38df25 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -234,30 +234,32 @@ pub fn intersect_permission_profiles( requested: PermissionProfile, granted: PermissionProfile, ) -> PermissionProfile { + fn intersect_permission_paths( + requested_paths: Option>, + granted_paths: Vec, + ) -> Option> { + requested_paths.and_then(|requested_paths| { + let requested_was_explicit_empty = requested_paths.is_empty(); + let intersected = requested_paths + .into_iter() + .filter(|path| granted_paths.contains(path)) + .collect::>(); + (requested_was_explicit_empty || !intersected.is_empty()).then_some(intersected) + }) + } + let file_system = requested .file_system .map(|requested_file_system| { let granted_file_system = granted.file_system.unwrap_or_default(); - let read = requested_file_system - .read - .map(|requested_read| { - let granted_read = granted_file_system.read.unwrap_or_default(); - requested_read - .into_iter() - .filter(|path| granted_read.contains(path)) - .collect() - }) - .filter(|paths: &Vec<_>| !paths.is_empty()); - let write = requested_file_system - .write - .map(|requested_write| { - let granted_write = granted_file_system.write.unwrap_or_default(); - requested_write - .into_iter() - .filter(|path| granted_write.contains(path)) - .collect() - }) - .filter(|paths: &Vec<_>| !paths.is_empty()); + let read = intersect_permission_paths( + requested_file_system.read, + granted_file_system.read.unwrap_or_default(), + ); + let write = intersect_permission_paths( + requested_file_system.write, + granted_file_system.write.unwrap_or_default(), + ); FileSystemPermissions { read, write } }) .filter(|file_system| !file_system.is_empty()); diff --git a/codex-rs/core/src/sandboxing/mod_tests.rs b/codex-rs/core/src/sandboxing/mod_tests.rs index 9a7a34e49d..bfc0341698 100644 --- a/codex-rs/core/src/sandboxing/mod_tests.rs +++ b/codex-rs/core/src/sandboxing/mod_tests.rs @@ -2,7 +2,6 @@ use super::EffectiveSandboxPermissions; use super::SandboxManager; use super::effective_file_system_sandbox_policy; -#[cfg(target_os = "macos")] use super::intersect_permission_profiles; use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; @@ -334,6 +333,33 @@ fn intersect_permission_profiles_preserves_default_macos_grants() { ); } +#[test] +fn intersect_permission_profiles_preserves_explicit_empty_read_lists() { + let requested_dir = AbsolutePathBuf::try_from("/tmp/requested").expect("absolute path"); + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(Vec::new()), + write: Some(vec![requested_dir.clone()]), + }), + ..Default::default() + }; + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(Vec::new()), + write: Some(vec![requested_dir.clone()]), + }), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted).file_system, + Some(FileSystemPermissions { + read: Some(Vec::new()), + write: Some(vec![requested_dir]), + }) + ); +} + #[cfg(target_os = "macos")] #[test] fn normalize_additional_permissions_preserves_macos_permissions() { diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 9e92a6b00f..7595d5074e 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -392,7 +392,9 @@ impl NetworkApprovalService { let mut cache_session_deny = false; let resolved = match approval_decision { - ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile + | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { PendingApprovalDecision::AllowOnce } ReviewDecision::ApprovedForSession => PendingApprovalDecision::AllowForSession, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 4b53ac156f..b9a432ce48 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -155,6 +155,7 @@ impl ToolOrchestrator { return Err(ToolError::Rejected(reason)); } ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => {} ReviewDecision::NetworkPolicyAmendment { @@ -309,6 +310,7 @@ impl ToolOrchestrator { return Err(ToolError::Rejected(reason)); } ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => {} ReviewDecision::NetworkPolicyAmendment { diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index f1e9912bc5..8137a4671f 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -54,6 +54,32 @@ impl ApplyPatchRuntime { Self } + fn execution_sandbox_permissions(req: &ApplyPatchRequest) -> SandboxPermissions { + if req.permissions_preapproved + && matches!( + req.sandbox_permissions, + SandboxPermissions::WithAdditionalPermissions + ) + { + SandboxPermissions::UseDefault + } else { + req.sandbox_permissions + } + } + + fn execution_additional_permissions(req: &ApplyPatchRequest) -> Option { + if req.permissions_preapproved + && matches!( + req.sandbox_permissions, + SandboxPermissions::WithAdditionalPermissions + ) + { + None + } else { + req.additional_permissions.clone() + } + } + fn build_guardian_review_request( req: &ApplyPatchRequest, call_id: &str, @@ -97,8 +123,8 @@ impl ApplyPatchRuntime { capture_policy: ExecCapturePolicy::ShellTool, // Run apply_patch with a minimal environment for determinism and to avoid leaks. env: HashMap::new(), - sandbox_permissions: req.sandbox_permissions, - additional_permissions: req.additional_permissions.clone(), + sandbox_permissions: Self::execution_sandbox_permissions(req), + additional_permissions: Self::execution_additional_permissions(req), justification: None, }) } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index d2812b5ecf..fea0df3681 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -68,3 +68,36 @@ fn guardian_review_request_includes_patch_context() { } ); } + +#[test] +fn build_command_spec_downgrades_preapproved_additional_permissions_to_default_sandbox() { + let path = std::env::temp_dir().join("apply-patch-preapproved.txt"); + let action = ApplyPatchAction::new_add_for_test(&path, "hello".to_string()); + let request = ApplyPatchRequest { + action, + file_paths: vec![ + AbsolutePathBuf::from_absolute_path(&path).expect("temp path should be absolute"), + ], + changes: HashMap::from([( + path, + FileChange::Add { + content: "hello".to_string(), + }, + )]), + exec_approval_requirement: ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, + additional_permissions: None, + permissions_preapproved: true, + timeout_ms: None, + codex_exe: None, + }; + + let spec = ApplyPatchRuntime::build_command_spec(&request, std::path::Path::new("/tmp")) + .expect("spec should build"); + + assert_eq!(spec.sandbox_permissions, SandboxPermissions::UseDefault); + assert_eq!(spec.additional_permissions, None); +} diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 948018dae6..645c6969da 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -542,6 +542,7 @@ impl CoreShellActionProvider { .await? { ReviewDecision::Approved + | ReviewDecision::ApprovedPersistToProfile | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { if needs_escalation { EscalationDecision::escalate(escalation_execution.clone()) diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 6df93bcd85..accf87da0b 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -561,15 +561,7 @@ impl TestCodexBuilder { for hook in self.pre_build_hooks.drain(..) { hook(home.path()); } - if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { - config.codex_linux_sandbox_exe = Some(path); - } else if let Ok(exe) = std::env::current_exe() - && let Some(path) = exe - .parent() - .and_then(|parent| parent.parent()) - .map(|parent| parent.join("codex")) - && path.is_file() - { + if let Some(path) = find_codex_cli_exe() { config.codex_linux_sandbox_exe = Some(path); } @@ -590,6 +582,20 @@ impl TestCodexBuilder { } } +fn find_codex_cli_exe() -> Option { + codex_utils_cargo_bin::cargo_bin("codex") + .ok() + .filter(|path| path.file_stem().is_some_and(|stem| stem == "codex")) + .or_else(|| { + std::env::current_exe().ok().and_then(|exe| { + exe.parent() + .and_then(|parent| parent.parent()) + .map(|parent| parent.join(format!("codex{}", std::env::consts::EXE_SUFFIX))) + .filter(|path| path.is_file()) + }) + }) +} + fn ensure_test_model_catalog(config: &mut Config) -> Result<()> { if config.model.as_deref() != Some(TEST_MODEL_WITH_EXPERIMENTAL_TOOLS) || config.model_catalog.is_some() diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 5f7e50f061..7d179f5ad1 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -1,6 +1,7 @@ // Aggregates all former standalone integration tests as modules. use std::ffi::OsString; +use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; use codex_arg0::Arg0PathEntryGuard; use codex_arg0::arg0_dispatch; use ctor::ctor; @@ -20,6 +21,25 @@ const CODEX_HOME_ENV_VAR: &str = "CODEX_HOME"; // NOTE: this doesn't work on ARM #[ctor] pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { + if std::env::args().nth(1).as_deref() == Some(CODEX_CORE_APPLY_PATCH_ARG1) { + let patch_arg = std::env::args().nth(2); + let exit_code = match patch_arg { + Some(patch_arg) => { + let mut stdout = std::io::stdout(); + let mut stderr = std::io::stderr(); + match codex_apply_patch::apply_patch(&patch_arg, &mut stdout, &mut stderr) { + Ok(()) => 0, + Err(_) => 1, + } + } + None => { + eprintln!("Error: {CODEX_CORE_APPLY_PATCH_ARG1} requires a UTF-8 PATCH argument."); + 1 + } + }; + std::process::exit(exit_code); + } + #[allow(clippy::unwrap_used)] let codex_home = tempfile::Builder::new() .prefix("codex-core-tests") 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..ab86b1e83b 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. @@ -3175,6 +3178,10 @@ pub enum ReviewDecision { /// remainder of the session. ApprovedForSession, + /// User has approved this request and wants the requested filesystem + /// permissions persisted into a named permission profile. + ApprovedPersistToProfile, + /// User chose to persist a network policy rule (allow/deny) for future /// requests to the same host. NetworkPolicyAmendment { @@ -3199,6 +3206,7 @@ impl ReviewDecision { ReviewDecision::Approved => "approved", ReviewDecision::ApprovedExecpolicyAmendment { .. } => "approved_with_amendment", ReviewDecision::ApprovedForSession => "approved_for_session", + ReviewDecision::ApprovedPersistToProfile => "approved_persist_to_profile", ReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => match network_policy_amendment.action { @@ -3211,6 +3219,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, } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4aa51df21c..80e9294bc1 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1658,6 +1658,7 @@ impl App { call_id: ev.call_id.clone(), reason: ev.reason.clone(), permissions: ev.permissions.clone(), + permissions_profile_persistence: ev.permissions_profile_persistence.clone(), }, )), _ => None, diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 1b403c251f..78fcc60972 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -28,8 +28,10 @@ use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::Op; +use codex_protocol::protocol::PersistPermissionProfileAction; use codex_protocol::protocol::ReviewDecision; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::PermissionProfilePersistence; use codex_protocol::request_permissions::RequestPermissionProfile; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -62,6 +64,7 @@ pub(crate) enum ApprovalRequest { call_id: String, reason: Option, permissions: RequestPermissionProfile, + permissions_profile_persistence: Option, }, ApplyPatch { thread_id: ThreadId, @@ -168,8 +171,11 @@ impl ApprovalOverlay { }, ), ), - ApprovalRequest::Permissions { .. } => ( - permissions_options(), + ApprovalRequest::Permissions { + permissions_profile_persistence, + .. + } => ( + permissions_options(permissions_profile_persistence.as_ref()), "Would you like to grant these permissions?".to_string(), ), ApprovalRequest::ApplyPatch { .. } => ( @@ -226,10 +232,16 @@ impl ApprovalOverlay { ApprovalRequest::Permissions { call_id, permissions, + permissions_profile_persistence, .. }, ApprovalDecision::Review(decision), - ) => self.handle_permissions_decision(call_id, permissions, decision.clone()), + ) => self.handle_permissions_decision( + call_id, + permissions, + permissions_profile_persistence.as_ref(), + decision.clone(), + ), (ApprovalRequest::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => { self.handle_patch_decision(id, decision.clone()); } @@ -278,13 +290,16 @@ impl ApprovalOverlay { &self, call_id: &str, permissions: &RequestPermissionProfile, + permissions_profile_persistence: Option<&PermissionProfilePersistence>, decision: ReviewDecision, ) { let Some(request) = self.current_request.as_ref() else { return; }; let granted_permissions = match decision { - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => permissions.clone(), + ReviewDecision::Approved + | ReviewDecision::ApprovedForSession + | ReviewDecision::ApprovedPersistToProfile => permissions.clone(), ReviewDecision::Denied | ReviewDecision::Abort => Default::default(), ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => Default::default(), @@ -307,6 +322,11 @@ impl ApprovalOverlay { ))); } let thread_id = request.thread_id(); + let persist_permissions = persist_permissions_for_permissions_decision( + &decision, + permissions_profile_persistence, + &granted_permissions, + ); self.app_event_tx.send(AppEvent::SubmitThreadOp { thread_id, op: Op::RequestPermissionsResponse { @@ -315,6 +335,7 @@ impl ApprovalOverlay { permissions: granted_permissions, scope, }, + persist_permissions, }, }); } @@ -443,9 +464,15 @@ impl BottomPaneView for ApprovalOverlay { ApprovalRequest::Permissions { call_id, permissions, + permissions_profile_persistence, .. } => { - self.handle_permissions_decision(call_id, permissions, ReviewDecision::Abort); + self.handle_permissions_decision( + call_id, + permissions, + permissions_profile_persistence.as_ref(), + ReviewDecision::Abort, + ); } ApprovalRequest::ApplyPatch { id, .. } => { self.handle_patch_decision(id, ReviewDecision::Abort); @@ -709,6 +736,7 @@ fn exec_options( display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], }), + ReviewDecision::ApprovedPersistToProfile => None, ReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => { @@ -855,8 +883,10 @@ fn patch_options() -> Vec { ] } -fn permissions_options() -> Vec { - vec![ +fn permissions_options( + permissions_profile_persistence: Option<&PermissionProfilePersistence>, +) -> Vec { + let mut options = vec![ ApprovalOption { label: "Yes, grant these permissions".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Approved), @@ -875,7 +905,35 @@ fn permissions_options() -> Vec { display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], }, - ] + ]; + if permissions_profile_persistence.is_some() { + options.insert( + 1, + ApprovalOption { + label: "Yes, always allow these permissions".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedPersistToProfile), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + }, + ); + } + options +} + +fn persist_permissions_for_permissions_decision( + decision: &ReviewDecision, + permissions_profile_persistence: Option<&PermissionProfilePersistence>, + granted_permissions: &RequestPermissionProfile, +) -> Option { + if !matches!(decision, ReviewDecision::ApprovedPersistToProfile) { + return None; + } + + let profile_name = permissions_profile_persistence?.profile_name.clone(); + Some(PersistPermissionProfileAction { + profile_name, + permissions: granted_permissions.clone().into(), + }) } fn elicitation_options() -> Vec { @@ -977,6 +1035,31 @@ mod tests { write: Some(vec![absolute_path("/tmp/out.txt")]), }), }, + permissions_profile_persistence: None, + } + } + + fn make_persistable_permissions_request() -> ApprovalRequest { + let ApprovalRequest::Permissions { + thread_id, + thread_label, + call_id, + reason, + permissions, + .. + } = make_permissions_request() + else { + unreachable!("permissions request"); + }; + ApprovalRequest::Permissions { + thread_id, + thread_label, + call_id, + reason, + permissions, + permissions_profile_persistence: Some(PermissionProfilePersistence { + profile_name: "workspace".to_string(), + }), } } @@ -1273,7 +1356,7 @@ mod tests { #[test] fn permissions_options_use_expected_labels() { - let labels: Vec = permissions_options() + let labels: Vec = permissions_options(None) .into_iter() .map(|option| option.label) .collect(); @@ -1440,6 +1523,20 @@ mod tests { ); } + #[test] + fn persistable_permissions_prompt_snapshot() { + let view = ApprovalOverlay::new( + make_persistable_permissions_request(), + AppEventSender::new(unbounded_channel::().0), + Features::with_defaults(), + ); + + assert_snapshot!( + "approval_overlay_persistable_permissions_prompt", + normalize_snapshot_paths(render_overlay_lines(&view, 60)) + ); + } + #[test] fn network_exec_prompt_title_includes_host() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_persistable_permissions_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_persistable_permissions_prompt.snap new file mode 100644 index 0000000000..5fd13c94ef --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_persistable_permissions_prompt.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/bottom_pane/approval_overlay.rs +expression: "normalize_snapshot_paths(render_overlay_lines(&view, 60))" +--- + + Would you like to grant these permissions? + + Reason: need workspace access + + Permission rule: network; read `/tmp/readme.txt`; write + `/tmp/out.txt` + +› 1. Yes, grant these permissions (y) + 2. Yes, always allow these permissions (p) + 3. Yes, grant these permissions for this session (a) + 4. No, continue without permissions (n) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c8d9e3b61b..7bfa563e4b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3445,6 +3445,7 @@ impl ChatWidget { call_id: ev.call_id, reason: ev.reason, permissions: ev.permissions, + permissions_profile_persistence: ev.permissions_profile_persistence, }; self.bottom_pane .push_approval_request(request, &self.config.features); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 8192724da0..8cb735546b 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -840,6 +840,18 @@ pub fn new_approval_decision_cell( ], ) } + ApprovedPersistToProfile => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✔ ".green(), + vec![ + actor.subject().into(), + "approved".bold(), + " codex to persist permissions for ".into(), + snippet, + ], + ) + } NetworkPolicyAmendment { network_policy_amendment, } => match network_policy_amendment.action { diff --git a/codex-rs/tui_app_server/src/app/app_server_requests.rs b/codex-rs/tui_app_server/src/app/app_server_requests.rs index 4381e883c0..365f5cf980 100644 --- a/codex-rs/tui_app_server/src/app/app_server_requests.rs +++ b/codex-rs/tui_app_server/src/app/app_server_requests.rs @@ -265,6 +265,7 @@ fn file_change_decision(decision: &ReviewDecision) -> Result Ok(FileChangeApprovalDecision::Accept), ReviewDecision::ApprovedForSession => Ok(FileChangeApprovalDecision::AcceptForSession), + ReviewDecision::ApprovedPersistToProfile => Ok(FileChangeApprovalDecision::Accept), ReviewDecision::Denied => Ok(FileChangeApprovalDecision::Decline), ReviewDecision::Abort => Ok(FileChangeApprovalDecision::Cancel), ReviewDecision::ApprovedExecpolicyAmendment { .. } => { @@ -383,6 +384,7 @@ mod tests { .expect("valid permissions"), scope: codex_protocol::request_permissions::PermissionGrantScope::Session, }, + persist_permissions: None, }) .expect("permissions response should serialize") .expect("permissions request should be pending"); diff --git a/codex-rs/tui_app_server/src/app_command.rs b/codex-rs/tui_app_server/src/app_command.rs index ed89ad86fb..abe58ae4cc 100644 --- a/codex-rs/tui_app_server/src/app_command.rs +++ b/codex-rs/tui_app_server/src/app_command.rs @@ -239,7 +239,11 @@ impl AppCommand { id: String, response: RequestPermissionsResponse, ) -> Self { - Self(Op::RequestPermissionsResponse { id, response }) + Self(Op::RequestPermissionsResponse { + id, + response, + persist_permissions: None, + }) } pub(crate) fn reload_user_config() -> Self { @@ -375,7 +379,7 @@ impl AppCommand { Op::UserInputAnswer { id, response } => { AppCommandView::UserInputAnswer { id, response } } - Op::RequestPermissionsResponse { id, response } => { + Op::RequestPermissionsResponse { id, response, .. } => { AppCommandView::RequestPermissionsResponse { id, response } } Op::ReloadUserConfig => AppCommandView::ReloadUserConfig, diff --git a/codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs b/codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs index f5d1cee621..52a7cf3365 100644 --- a/codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs @@ -279,7 +279,9 @@ impl ApprovalOverlay { return; }; let granted_permissions = match decision { - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => permissions.clone(), + ReviewDecision::Approved + | ReviewDecision::ApprovedForSession + | ReviewDecision::ApprovedPersistToProfile => permissions.clone(), ReviewDecision::Denied | ReviewDecision::Abort => Default::default(), ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => Default::default(), @@ -695,6 +697,7 @@ fn exec_options( display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], }), + ReviewDecision::ApprovedPersistToProfile => None, ReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => { diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 82cd0d99bc..03ad4bfb8c 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -1391,6 +1391,7 @@ fn request_permissions_from_params( serde_json::to_value(params.permissions).unwrap_or(serde_json::Value::Null), ) .unwrap_or_default(), + permissions_profile_persistence: None, } } diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index 38937406b7..a57e80293c 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -846,6 +846,18 @@ pub fn new_approval_decision_cell( ], ) } + ApprovedPersistToProfile => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✔ ".green(), + vec![ + actor.subject().into(), + "approved".bold(), + " codex to persist permissions for ".into(), + snippet, + ], + ) + } NetworkPolicyAmendment { network_policy_amendment, } => match network_policy_amendment.action {