openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-24 18:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add request_permissions profile persistence core support

Browse Source 
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Dylan Hurd
2026-03-23 22:37:28 -07:00
co-authored by Codex
parent 2227248cd6
commit 98b2a8c7f3
37 changed files with 574 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json
+7
View File
@@ -65,6 +65,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the requested filesystem permissions persisted into a named permission profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json
+7
View File
@@ -65,6 +65,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the requested filesystem permissions persisted into a named permission profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
+7
View File
@@ -3368,6 +3368,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the requested filesystem permissions persisted into a named permission profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts
+1 -1
View File
@@ -7,4 +7,4 @@ import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
/**
* User's decision in response to an ExecApprovalRequest.
*/
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "abort";
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | "approved_persist_to_profile" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "abort";
codex-rs/app-server-protocol/src/protocol/v2.rs
+1
View File
@@ -1023,6 +1023,7 @@ impl From<CoreReviewDecision> for CommandExecutionApprovalDecision {
execpolicy_amendment: proposed_execpolicy_amendment.into(),
},
CoreReviewDecision::ApprovedForSession => Self::AcceptForSession,
CoreReviewDecision::ApprovedPersistToProfile => Self::Accept,
CoreReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => Self::ApplyNetworkPolicyAmendment {
codex-rs/app-server/src/bespoke_event_handling.rs
+2
View File
@@ -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
{
codex-rs/core/src/codex.rs
+39 -3
View File
@@ -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)]
@@ -3034,7 +3035,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()
@@ -4326,8 +4331,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 } => {
@@ -4473,6 +4488,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;
@@ -4494,6 +4510,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;
@@ -4788,7 +4805,26 @@ mod handlers {
sess: &Arc<Session>,
id: String,
response: RequestPermissionsResponse,
persist_permissions: Option<PersistPermissionProfileAction>,
) {
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;
}
codex-rs/core/src/codex_delegate.rs
+4
View File
@@ -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;
}
codex-rs/core/src/codex_delegate_tests.rs
+2
View File
@@ -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,
}
);
}
codex-rs/core/src/config/edit.rs
+57
View File
@@ -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;
@@ -58,6 +61,8 @@ pub enum ConfigEdit {
segments: Vec<String>,
value: TomlItem,
},
/// Persist filesystem-only permissions into a named profile.
PersistPermissionProfile(PersistPermissionProfileAction),
/// Remove the value stored at the exact dotted path.
ClearPath { segments: Vec<String> },
}
@@ -401,6 +406,9 @@ impl ConfigDocument {
Ok(self.set_skill_config(SkillConfigSelector::Name(name.clone()), *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
@@ -436,6 +444,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<String, McpServerConfig>) -> bool {
if servers.is_empty() {
return self.clear(Scope::Global, &["mcp_servers"]);
@@ -714,6 +739,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")
@@ -1022,6 +1073,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)
codex-rs/core/src/config/edit_tests.rs
+54
View File
@@ -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,57 @@ 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();
let read_path = absolute_path("/tmp/read");
let write_path = absolute_path("/tmp/write");
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![read_path.clone(), write_path.clone()]),
write: Some(vec![write_path.clone()]),
}),
..Default::default()
},
})
.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 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 set_model_availability_nux_count_writes_shown_count() {
let tmp = tempdir().expect("tmpdir");
codex-rs/core/src/config/mod.rs
+1
View File
@@ -129,6 +129,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;
codex-rs/core/src/config/permissions.rs
+46
View File
@@ -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<PermissionProfilePersistence> {
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,
codex-rs/core/src/mcp_tool_call.rs
+1
View File
@@ -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,
codex-rs/core/src/sandboxing/mod.rs
+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());
codex-rs/core/src/sandboxing/mod_tests.rs
+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() {
codex-rs/core/src/tools/network_approval.rs
+3 -1
View File
@@ -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,
codex-rs/core/src/tools/orchestrator.rs
+2
View File
@@ -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 {
codex-rs/core/src/tools/runtimes/apply_patch.rs
+28 -2
View File
@@ -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,
})
}
codex-rs/core/src/tools/runtimes/apply_patch_tests.rs
+33
View File
@@ -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);
}
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+1
View File
@@ -542,6 +542,7 @@ impl CoreShellActionProvider {
.await?
{
ReviewDecision::Approved
| ReviewDecision::ApprovedPersistToProfile
| ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
if needs_escalation {
EscalationDecision::escalate(escalation_execution.clone())
codex-rs/core/tests/common/test_codex.rs
+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()
codex-rs/core/tests/suite/mod.rs
+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")
codex-rs/core/tests/suite/request_permissions.rs
+7
View File
@@ -1088,6 +1088,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?;
@@ -1202,6 +1203,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
persist_permissions: None,
})
.await?;
@@ -1315,6 +1317,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?;
@@ -1424,6 +1427,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?;
@@ -1570,6 +1574,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
permissions: granted_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
persist_permissions: None,
})
.await?;
@@ -1688,6 +1693,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;
@@ -1805,6 +1811,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;
codex-rs/core/tests/suite/request_permissions_tool.rs
+2
View File
@@ -262,6 +262,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
persist_permissions: None,
})
.await?;
@@ -381,6 +382,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_apply_patch_wi
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
persist_permissions: None,
})
.await?;
codex-rs/protocol/src/protocol.rs
+14
View File
@@ -415,6 +415,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.
@@ -3250,6 +3253,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 {
@@ -3274,6 +3281,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 {
@@ -3286,6 +3294,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")]
codex-rs/protocol/src/request_permissions.rs
+8
View File
@@ -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>,
}
codex-rs/tui/src/app.rs
+1
View File
@@ -1808,6 +1808,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,
codex-rs/tui/src/bottom_pane/approval_overlay.rs
+106 -9
View File
@@ -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<String>,
permissions: RequestPermissionProfile,
permissions_profile_persistence: Option<PermissionProfilePersistence>,
},
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<ApprovalOption> {
]
}
fn permissions_options() -> Vec<ApprovalOption> {
vec![
fn permissions_options(
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Vec<ApprovalOption> {
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<ApprovalOption> {
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<PersistPermissionProfileAction> {
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<ApprovalOption> {
@@ -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<String> = permissions_options()
let labels: Vec<String> = 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::<AppEvent>().0),
Features::with_defaults(),
);
assert_snapshot!(
"approval_overlay_persistable_permissions_prompt",
normalize_snapshot_paths(render_overlay_lines(&view, 72))
);
}
#[test]
fn network_exec_prompt_title_includes_host() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_persistable_permissions_prompt.snap
+18
View File
@@ -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
codex-rs/tui/src/chatwidget.rs
+1
View File
@@ -3534,6 +3534,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);
codex-rs/tui/src/history_cell.rs
+12
View File
@@ -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 {
codex-rs/tui_app_server/src/app/app_server_requests.rs
+2
View File
@@ -265,6 +265,7 @@ fn file_change_decision(decision: &ReviewDecision) -> Result<FileChangeApprovalD
match decision {
ReviewDecision::Approved => 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");
codex-rs/tui_app_server/src/app_command.rs
+6 -2
View File
@@ -241,7 +241,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 {
@@ -379,7 +383,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,
codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs
+4 -1
View File
@@ -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,
} => {
codex-rs/tui_app_server/src/chatwidget.rs
+1
View File
@@ -1413,6 +1413,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,
}
}
codex-rs/tui_app_server/src/history_cell.rs
+12
View File
@@ -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 {