openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-01 19:33:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

unify permission preset logic

Browse Source
This commit is contained in:
Joey Trasatti
2026-06-22 15:25:01 -07:00
parent 7e3b5d3e62
commit 3f96615d59
10 changed files with 153 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/permission_presets.rs
+27 -18
View File
@@ -26,16 +26,26 @@ pub(crate) async fn permission_presets(
.load_for_cwd(None, ConfigOverrides::default(), cwd)
.await?;
let selectable_profiles = config
.selectable_permission_profiles()
.await?
.selectable_permission_profiles()?
.into_iter()
.map(|profile| (profile.id.clone(), profile))
.collect::<HashMap<_, _>>();
let active_profile_id = config
.permissions
.active_permission_profile()
.map(|profile| profile.id);
let mut candidates = Vec::new();
for preset in builtin_approval_presets() {
let Some(profile) = selectable_profiles.get(&preset.active_permission_profile.id) else {
if !selectable_profiles.contains_key(&preset.active_permission_profile.id) {
continue;
}
let profile_is_default = match active_profile_id.as_deref() {
Some(id) => id == preset.active_permission_profile.id,
None => preset.matches_permission_profile(
config.permissions.permission_profile(),
config.cwd.as_path(),
),
};
candidates.push(PermissionPresetCandidate {
id: preset.id.to_string(),
@@ -43,7 +53,7 @@ pub(crate) async fn permission_presets(
description: None,
approval_policy: preset.approval,
approvals_reviewer: ApprovalsReviewer::User,
profile_is_default: profile.is_default,
profile_is_default,
});
if preset.id == "auto" {
candidates.push(PermissionPresetCandidate {
@@ -58,7 +68,7 @@ pub(crate) async fn permission_presets(
mcp_elicitations: true,
}),
approvals_reviewer: ApprovalsReviewer::User,
profile_is_default: profile.is_default,
profile_is_default,
});
if config.features.enabled(Feature::GuardianApproval) {
candidates.push(PermissionPresetCandidate {
@@ -67,7 +77,7 @@ pub(crate) async fn permission_presets(
description: None,
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: ApprovalsReviewer::AutoReview,
profile_is_default: profile.is_default,
profile_is_default,
});
}
}
@@ -78,18 +88,17 @@ pub(crate) async fn permission_presets(
.filter(|profile| !profile.id.starts_with(':'))
.collect::<Vec<_>>();
custom_profiles.sort_by(|left, right| left.id.cmp(&right.id));
candidates.extend(
custom_profiles
.into_iter()
.map(|profile| PermissionPresetCandidate {
id: format!("permission-profile:{}", profile.id),
permission_profile_id: profile.id,
description: profile.description,
approval_policy: config.permissions.approval_policy.value(),
approvals_reviewer: config.approvals_reviewer,
profile_is_default: profile.is_default,
}),
);
candidates.extend(custom_profiles.into_iter().map(|profile| {
let profile_is_default = active_profile_id.as_deref() == Some(profile.id.as_str());
PermissionPresetCandidate {
id: format!("permission-profile:{}", profile.id),
permission_profile_id: profile.id,
description: profile.description,
approval_policy: config.permissions.approval_policy.value(),
approvals_reviewer: config.approvals_reviewer,
profile_is_default,
}
}));
let requirements = config.config_layer_stack.requirements();
let mut presets = Vec::new();
codex-rs/app-server/tests/suite/v2/permission_preset_list.rs
+62 -108
View File
@@ -172,128 +172,82 @@ description = "Inspect without writes."
}
#[tokio::test]
async fn permission_preset_list_matches_legacy_read_only_as_default() -> Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("config.toml"),
r#"
async fn permission_preset_list_matches_legacy_defaults() -> Result<()> {
for (config, requirements, expected) in [
(
r#"
sandbox_mode = "read-only"
approval_policy = "on-request"
"#,
)?;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_permission_preset_list_request(PermissionPresetListParams {
cursor: None,
limit: None,
cwd: None,
})
.await?;
let actual = read_response::<PermissionPresetListResponse>(&mut mcp, request_id).await?;
assert_eq!(
actual
.data
.into_iter()
.filter(|preset| preset.is_default)
.collect::<Vec<_>>(),
vec![preset(
"read-only",
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
AskForApproval::OnRequest,
ApprovalsReviewer::User,
true,
)]
);
Ok(())
}
#[tokio::test]
async fn permission_preset_list_marks_managed_legacy_fallback_as_default() -> Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("config.toml"),
r#"
None,
preset(
"read-only",
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
AskForApproval::OnRequest,
ApprovalsReviewer::User,
true,
),
),
(
r#"
sandbox_mode = "workspace-write"
approval_policy = "on-request"
"#,
)?;
std::fs::write(
codex_home.path().join("requirements.toml"),
r#"
Some(
r#"
allowed_sandbox_modes = ["read-only"]
"#,
)?;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_permission_preset_list_request(PermissionPresetListParams {
cursor: None,
limit: None,
cwd: None,
})
.await?;
let actual = read_response::<PermissionPresetListResponse>(&mut mcp, request_id).await?;
assert_eq!(
actual
.data
.into_iter()
.filter(|preset| preset.is_default)
.collect::<Vec<_>>(),
vec![preset(
"read-only",
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
AskForApproval::OnRequest,
ApprovalsReviewer::User,
true,
)]
);
Ok(())
}
#[tokio::test]
async fn permission_preset_list_matches_legacy_full_access_as_default() -> Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("config.toml"),
r#"
),
preset(
"read-only",
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
AskForApproval::OnRequest,
ApprovalsReviewer::User,
true,
),
),
(
r#"
sandbox_mode = "danger-full-access"
approval_policy = "never"
"#,
)?;
None,
preset(
"full-access",
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
AskForApproval::Never,
ApprovalsReviewer::User,
true,
),
),
] {
let codex_home = TempDir::new()?;
std::fs::write(codex_home.path().join("config.toml"), config)?;
if let Some(requirements) = requirements {
std::fs::write(codex_home.path().join("requirements.toml"), requirements)?;
}
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_permission_preset_list_request(PermissionPresetListParams {
cursor: None,
limit: None,
cwd: None,
})
.await?;
let actual = read_response::<PermissionPresetListResponse>(&mut mcp, request_id).await?;
let request_id = mcp
.send_permission_preset_list_request(PermissionPresetListParams {
cursor: None,
limit: None,
cwd: None,
})
.await?;
let actual = read_response::<PermissionPresetListResponse>(&mut mcp, request_id).await?;
assert_eq!(
actual
.data
.into_iter()
.filter(|preset| preset.is_default)
.collect::<Vec<_>>(),
vec![preset(
"full-access",
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
AskForApproval::Never,
ApprovalsReviewer::User,
true,
)]
);
assert_eq!(
actual
.data
.into_iter()
.filter(|preset| preset.is_default)
.collect::<Vec<_>>(),
vec![expected]
);
}
Ok(())
}
codex-rs/core/src/config/config_tests.rs
-1
View File
@@ -2417,7 +2417,6 @@ async fn default_permissions_can_select_builtin_profile_without_permissions_tabl
.await?;
assert!(config.explicit_permission_profile_mode);
assert!(config.custom_permission_profiles.is_empty());
let policy = config.permissions.file_system_sandbox_policy();
assert_eq!(
config
codex-rs/core/src/config/mod.rs
-1
View File
@@ -3773,7 +3773,6 @@ impl Config {
windows_sandbox_private_desktop,
},
explicit_permission_profile_mode,
custom_permission_profiles,
approvals_reviewer: constrained_approvals_reviewer.value(),
enforce_residency: enforce_residency.value,
notify: cfg.notify,
codex-rs/core/src/config/selectable_permission_profiles.rs
+4 -38
View File
@@ -5,9 +5,7 @@ use super::permissions::BUILT_IN_READ_ONLY_PROFILE;
use super::permissions::BUILT_IN_WORKSPACE_PROFILE;
use super::permissions::builtin_permission_profile;
use super::permissions::compile_permission_profile_selection;
use crate::windows_sandbox::WindowsSandboxLevelExt;
use codex_config::config_toml::ConfigToml;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use std::io::ErrorKind;
@@ -15,12 +13,11 @@ use std::io::ErrorKind;
pub struct SelectablePermissionProfile {
pub id: String,
pub description: Option<String>,
pub is_default: bool,
}
impl Config {
/// Returns named permission profiles that can be selected under the current requirements.
pub async fn selectable_permission_profiles(
/// Returns built-in and configured permission profiles allowed by current requirements.
pub fn selectable_permission_profiles(
&self,
) -> std::io::Result<Vec<SelectablePermissionProfile>> {
let config_toml: ConfigToml = self
@@ -31,26 +28,6 @@ impl Config {
let requirements_toml = self.config_layer_stack.requirements_toml();
let permissions =
merge_managed_permission_profiles(config_toml.permissions.as_ref(), requirements_toml)?;
let active_profile_id = self
.permissions
.active_permission_profile()
.map(|profile| profile.id);
let requirements = self.config_layer_stack.requirements();
let default_permission_profile = if active_profile_id.is_none() {
let mut permission_profile = requirements.permission_profile.clone();
let configured_profile = config_toml
.derive_permission_profile(
/*sandbox_mode_override*/ None,
WindowsSandboxLevel::from_config(self),
Some(&self.active_project),
Some(&requirements.permission_profile),
)
.await;
let _ = permission_profile.set(configured_profile);
Some(permission_profile.get().clone())
} else {
None
};
let mut profiles = Vec::new();
for id in [
@@ -62,8 +39,6 @@ impl Config {
permissions.as_ref(),
id,
/*description*/ None,
active_profile_id.as_deref(),
default_permission_profile.as_ref(),
) {
profiles.push(profile);
}
@@ -74,8 +49,6 @@ impl Config {
Some(permissions),
id,
profile.description.clone(),
active_profile_id.as_deref(),
default_permission_profile.as_ref(),
) {
profiles.push(profile);
}
@@ -90,8 +63,6 @@ impl Config {
permissions: Option<&codex_config::permissions_toml::PermissionsToml>,
id: &str,
description: Option<String>,
active_profile_id: Option<&str>,
default_permission_profile: Option<&PermissionProfile>,
) -> Option<SelectablePermissionProfile> {
if self
.config_layer_stack
@@ -120,21 +91,16 @@ impl Config {
self.permissions
.can_set_permission_profile(&permission_profile)
.ok()?;
let mut constrained_permission_profile = self
.config_layer_stack
self.config_layer_stack
.requirements()
.permission_profile
.clone();
constrained_permission_profile
.clone()
.set(permission_profile)
.ok()?;
Some(SelectablePermissionProfile {
id: id.to_string(),
description,
is_default: active_profile_id == Some(id)
|| active_profile_id.is_none()
&& default_permission_profile == Some(constrained_permission_profile.get()),
})
}
}
codex-rs/thread-manager-sample/src/main.rs
-1
View File
@@ -185,7 +185,6 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
Constrained::allow_any(PermissionProfile::read_only()),
)?,
explicit_permission_profile_mode: false,
custom_permission_profiles: Vec::new(),
approvals_reviewer: ApprovalsReviewer::User,
enforce_residency: Constrained::allow_any(/*initial_value*/ None),
hide_agent_reasoning: false,
codex-rs/tui/src/chatwidget/permission_popups.rs
+10 -57
View File
@@ -25,6 +25,13 @@ impl ChatWidget {
let current_permission_profile = self.config.permissions.permission_profile().clone();
let guardian_approval_enabled = self.config.features.enabled(Feature::GuardianApproval);
let current_review_policy = self.config.approvals_reviewer;
let preset_is_current = |preset: &ApprovalPreset| {
current_approval == AskForApproval::from(preset.approval)
&& preset.matches_permission_profile(
&current_permission_profile,
self.config.cwd.as_path(),
)
};
let mut items: Vec<SelectionItem> = Vec::new();
let presets: Vec<ApprovalPreset> = builtin_approval_presets();
@@ -86,12 +93,7 @@ impl ChatWidget {
name: base_name.clone(),
description: base_description.clone(),
is_current: current_review_policy == ApprovalsReviewer::User
&& Self::preset_matches_current(
current_approval,
&current_permission_profile,
self.config.cwd.as_path(),
&preset,
),
&& preset_is_current(&preset),
actions: default_actions,
dismiss_on_select: true,
disabled_reason: default_disabled_reason,
@@ -103,12 +105,7 @@ impl ChatWidget {
name: APPROVE_FOR_ME_LABEL.to_string(),
description: Some(AUTO_REVIEW_DESCRIPTION.to_string()),
is_current: current_review_policy == ApprovalsReviewer::AutoReview
&& Self::preset_matches_current(
current_approval,
&current_permission_profile,
self.config.cwd.as_path(),
&preset,
),
&& preset_is_current(&preset),
actions: self.permission_mode_actions(
&preset,
APPROVE_FOR_ME_LABEL.to_string(),
@@ -126,12 +123,7 @@ impl ChatWidget {
items.push(SelectionItem {
name: base_name,
description: base_description,
is_current: Self::preset_matches_current(
current_approval,
&current_permission_profile,
self.config.cwd.as_path(),
&preset,
),
is_current: preset_is_current(&preset),
actions: default_actions,
dismiss_on_select: true,
disabled_reason: default_disabled_reason,
@@ -365,45 +357,6 @@ impl ChatWidget {
apply_actions()
}
pub(super) fn preset_matches_current(
current_approval: AskForApproval,
current_permission_profile: &PermissionProfile,
cwd: &std::path::Path,
preset: &ApprovalPreset,
) -> bool {
let preset_approval = AskForApproval::from(preset.approval);
if current_approval != preset_approval {
return false;
}
match preset.id {
"full-access" => matches!(current_permission_profile, PermissionProfile::Disabled),
"read-only" => {
let file_system_policy = current_permission_profile.file_system_sandbox_policy();
matches!(
current_permission_profile,
PermissionProfile::Managed { .. }
) && !file_system_policy.has_full_disk_write_access()
&& file_system_policy
.get_writable_roots_with_cwd(cwd)
.is_empty()
&& current_permission_profile.network_sandbox_policy()
== preset.permission_profile.network_sandbox_policy()
}
"auto" => {
let file_system_policy = current_permission_profile.file_system_sandbox_policy();
matches!(
current_permission_profile,
PermissionProfile::Managed { .. }
) && file_system_policy.can_write_path_with_cwd(cwd, cwd)
&& !file_system_policy.has_full_disk_write_access()
&& current_permission_profile.network_sandbox_policy()
== preset.permission_profile.network_sandbox_policy()
}
_ => current_permission_profile == &preset.permission_profile,
}
}
pub(crate) fn open_full_access_confirmation(
&mut self,
preset: ApprovalPreset,
codex-rs/tui/src/chatwidget/permissions_menu.rs
+9 -11
View File
@@ -27,17 +27,15 @@ impl ChatWidget {
);
return;
};
let mut items = vec![
self.builtin_permission_mode_selection_item(
default,
":workspace",
default
.description
.replace(" (Identical to Agent mode)", ""),
AskForApproval::from(default.approval),
ApprovalsReviewer::User,
),
];
let mut items = vec![self.builtin_permission_mode_selection_item(
default,
":workspace",
default
.description
.replace(" (Identical to Agent mode)", ""),
AskForApproval::from(default.approval),
ApprovalsReviewer::User,
)];
if self.config.features.enabled(Feature::GuardianApproval) {
items.push(self.builtin_permission_mode_selection_item(
default,
codex-rs/tui/src/chatwidget/tests/permissions.rs
+2 -21
View File
@@ -326,23 +326,9 @@ async fn preset_matching_accepts_workspace_write_with_extra_roots() {
let cwd = test_path_buf("/tmp/project").abs();
assert!(
ChatWidget::preset_matches_current(
AskForApproval::OnRequest,
&current_profile,
cwd.as_path(),
&preset
),
preset.matches_permission_profile(&current_profile, cwd.as_path()),
"WorkspaceWrite with extra roots should still match the Ask for approval preset"
);
assert!(
!ChatWidget::preset_matches_current(
AskForApproval::Never,
&current_profile,
cwd.as_path(),
&preset
),
"approval mismatch should prevent matching the preset"
);
}
#[tokio::test]
@@ -374,12 +360,7 @@ async fn preset_matching_does_not_treat_non_cwd_writable_profile_as_read_only()
let cwd = test_path_buf("/tmp/project").abs();
assert!(
!ChatWidget::preset_matches_current(
AskForApproval::OnRequest,
&current_profile,
cwd.as_path(),
&preset
),
!preset.matches_permission_profile(&current_profile, cwd.as_path()),
"profiles with any writable root should not be classified as Read Only"
);
}
codex-rs/utils/approval-presets/src/lib.rs
+39
View File
@@ -4,6 +4,7 @@ use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use std::path::Path;
/// A simple preset pairing an approval policy with a permission profile.
#[derive(Debug, Clone)]
@@ -22,6 +23,44 @@ pub struct ApprovalPreset {
pub permission_profile: PermissionProfile,
}
impl ApprovalPreset {
/// Whether the current permission profile has the same user-visible access as this preset.
pub fn matches_permission_profile(
&self,
current_permission_profile: &PermissionProfile,
cwd: &Path,
) -> bool {
match self.active_permission_profile.id.as_str() {
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS => {
matches!(current_permission_profile, PermissionProfile::Disabled)
}
BUILT_IN_PERMISSION_PROFILE_READ_ONLY => {
let file_system_policy = current_permission_profile.file_system_sandbox_policy();
matches!(
current_permission_profile,
PermissionProfile::Managed { .. }
) && !file_system_policy.has_full_disk_write_access()
&& file_system_policy
.get_writable_roots_with_cwd(cwd)
.is_empty()
&& current_permission_profile.network_sandbox_policy()
== self.permission_profile.network_sandbox_policy()
}
BUILT_IN_PERMISSION_PROFILE_WORKSPACE => {
let file_system_policy = current_permission_profile.file_system_sandbox_policy();
matches!(
current_permission_profile,
PermissionProfile::Managed { .. }
) && file_system_policy.can_write_path_with_cwd(cwd, cwd)
&& !file_system_policy.has_full_disk_write_access()
&& current_permission_profile.network_sandbox_policy()
== self.permission_profile.network_sandbox_policy()
}
_ => current_permission_profile == &self.permission_profile,
}
}
}
/// Built-in list of approval presets that pair approval and permissions.
///
/// Keep this UI-agnostic so it can be reused by both TUI and MCP server.