Add request_permissions profile persistence core support

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Dylan Hurd
2026-03-23 14:51:13 -07:00
co-authored by Codex
parent b0236501e2
commit 48fd30375c
20 changed files with 397 additions and 37 deletions
@@ -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
{
+32 -3
View File
@@ -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<Session>, config: Arc<Config>, 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<Session>,
id: String,
response: RequestPermissionsResponse,
persist_permissions: Option<PersistPermissionProfileAction>,
) {
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;
}
+1
View File
@@ -764,6 +764,7 @@ async fn handle_request_permissions(
.submit(Op::RequestPermissionsResponse {
id: call_id,
response,
persist_permissions: None,
})
.await;
}
@@ -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,
}
);
}
+1
View File
@@ -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;
@@ -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<PermissionProfilePersistence> {
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<ConfigEdit> {
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:?}"),
}
}
}
+22 -20
View File
@@ -234,30 +234,32 @@ pub fn intersect_permission_profiles(
requested: PermissionProfile,
granted: PermissionProfile,
) -> PermissionProfile {
fn intersect_permission_paths(
requested_paths: Option<Vec<AbsolutePathBuf>>,
granted_paths: Vec<AbsolutePathBuf>,
) -> Option<Vec<AbsolutePathBuf>> {
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::<Vec<_>>();
(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());
+27 -1
View File
@@ -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() {
@@ -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<PermissionProfile> {
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,
})
}
@@ -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);
}
+15 -9
View File
@@ -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<PathBuf> {
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()
+20
View File
@@ -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")
@@ -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;
@@ -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?;
+9
View File
@@ -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<PersistPermissionProfileAction>,
},
/// 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")]
@@ -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<String>,
pub permissions: RequestPermissionProfile,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub permissions_profile_persistence: Option<PermissionProfilePersistence>,
}
@@ -315,6 +315,7 @@ impl ApprovalOverlay {
permissions: granted_permissions,
scope,
},
persist_permissions: None,
},
});
}
@@ -383,6 +383,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");
+6 -2
View File
@@ -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,
@@ -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,
}
}