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

gracefully handle failures, inherited profiles

Browse Source
This commit is contained in:
Dylan Hurd
2026-03-17 09:46:28 -07:00
parent 9ab3a31630
commit fd9d4a9090
6 changed files with 192 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+2 -1
View File
@@ -3149,7 +3149,7 @@ impl Session {
pub async fn notify_request_permissions_response(
&self,
call_id: &str,
response: RequestPermissionsResponse,
mut response: RequestPermissionsResponse,
) {
let mut granted_for_session = None;
let mut granted_always_allow = None;
@@ -3192,6 +3192,7 @@ impl Session {
.await
{
error!("failed to persist granted permissions: {err}");
response.scope = PermissionGrantScope::Session;
} else {
self.reload_user_config_layer().await;
}
codex-rs/core/src/codex_tests.rs
+2 -2
View File
@@ -2613,7 +2613,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
}
#[tokio::test]
async fn notify_request_permissions_response_persists_always_allow_permissions() {
async fn notify_request_permissions_response_falls_back_to_session_when_persist_fails() {
let (session, _turn_context) = make_session_and_context().await;
let active_turn = ActiveTurn::default();
let (tx, rx) = oneshot::channel();
@@ -2654,7 +2654,7 @@ async fn notify_request_permissions_response_persists_always_allow_permissions()
);
assert_eq!(
rx.await.expect("response should be delivered").scope,
PermissionGrantScope::AlwaysAllow
PermissionGrantScope::Session
);
assert!(!session.codex_home().await.join(CONFIG_TOML_FILE).exists());
codex-rs/core/src/config/config_tests.rs
+113 -1
View File
@@ -6220,9 +6220,121 @@ sandbox_mode = "workspace-write"
assert_eq!(
err.to_string(),
"cannot persist granted permissions without `default_permissions`"
"cannot persist granted permissions without user-configured `default_permissions`"
);
assert!(!codex_home.path().join(CONFIG_TOML_FILE).exists());
Ok(())
}
#[tokio::test]
async fn persist_granted_permission_profile_does_not_copy_inherited_profiles() -> anyhow::Result<()>
{
let codex_home = TempDir::new()?;
let cwd = TempDir::new()?;
std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?;
std::fs::create_dir_all(cwd.path().join(".codex"))?;
let inherited_read = AbsolutePathBuf::try_from(cwd.path().join("repo-only"))?;
std::fs::write(
cwd.path().join(".codex").join(CONFIG_TOML_FILE),
format!(
r#"
sandbox_mode = "workspace-write"
default_permissions = "workspace"
[permissions.workspace.filesystem]
read = ["{}"]
[permissions.workspace.network]
enabled = true
"#,
inherited_read.display()
),
)?;
std::fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
r#"
sandbox_mode = "workspace-write"
default_permissions = "workspace"
[permissions.workspace]
"#,
)?;
let cwd = AbsolutePathBuf::try_from(cwd.path().to_path_buf())?;
let config_layer_stack = load_config_layers_state(
codex_home.path(),
Some(cwd.clone()),
&[],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
)
.await?;
let config_toml = deserialize_config_toml_with_base(
config_layer_stack.effective_config(),
codex_home.path(),
)?;
let config = Config::load_config_with_layer_stack(
config_toml,
ConfigOverrides {
cwd: Some(cwd.to_path_buf()),
..Default::default()
},
codex_home.path().to_path_buf(),
config_layer_stack,
)?;
let granted_write = AbsolutePathBuf::try_from(cwd.as_path().join("logs"))?;
let profile_name = persist_granted_permission_profile(
codex_home.path(),
&config,
&PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![granted_write.clone()]),
}),
network: None,
macos: None,
},
)
.await?;
assert_eq!(profile_name, "workspace");
let persisted = std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE))?;
let persisted: ConfigToml = toml::from_str(&persisted)?;
assert_eq!(persisted.default_permissions.as_deref(), Some("workspace"));
assert_eq!(
persisted
.permissions
.as_ref()
.and_then(|permissions| permissions.entries.get("workspace")),
Some(&PermissionProfileToml {
filesystem: Some(FilesystemPermissionsToml {
entries: BTreeMap::from([(
granted_write.display().to_string(),
FilesystemPermissionToml::Access(FileSystemAccessMode::Write),
)]),
}),
network: None,
macos: None,
})
);
assert!(
!persisted
.permissions
.as_ref()
.and_then(|permissions| permissions.entries.get("workspace"))
.is_some_and(
|profile| profile
.filesystem
.as_ref()
.is_some_and(|filesystem| filesystem
.entries
.contains_key(&inherited_read.display().to_string()))
)
);
Ok(())
}
codex-rs/core/src/config/mod.rs
+11 -9
View File
@@ -2877,12 +2877,12 @@ impl Config {
}
}
pub(crate) fn default_permissions_profile_name(config: &Config) -> Option<String> {
pub(crate) fn user_default_permissions_profile_name(config: &Config) -> Option<String> {
config
.config_layer_stack
.effective_config()
.try_into::<PermissionSelectionToml>()
.ok()
.get_user_layer()
.map(|layer| layer.config.clone())
.and_then(|config| config.try_into::<PermissionSelectionToml>().ok())
.and_then(|selection| selection.default_permissions)
}
@@ -2892,14 +2892,16 @@ pub(crate) async fn persist_granted_permission_profile(
granted_permissions: &PermissionProfile,
) -> anyhow::Result<String> {
let granted_profile = permission_profile_toml_from_runtime_permissions(granted_permissions);
let Some(profile_name) = default_permissions_profile_name(config) else {
anyhow::bail!("cannot persist granted permissions without `default_permissions`");
let Some(profile_name) = user_default_permissions_profile_name(config) else {
anyhow::bail!(
"cannot persist granted permissions without user-configured `default_permissions`"
);
};
let existing = config
.config_layer_stack
.effective_config()
.try_into::<ConfigToml>()
.ok()
.get_user_layer()
.map(|layer| layer.config.clone())
.and_then(|config| config.try_into::<ConfigToml>().ok())
.and_then(|config_toml| config_toml.permissions)
.and_then(|permissions| permissions.entries.get(&profile_name).cloned());
let profile = merge_permission_profile_toml(existing.as_ref(), &granted_profile);
codex-rs/tui/src/bottom_pane/approval_overlay.rs
+32 -8
View File
@@ -713,14 +713,7 @@ fn exec_options(
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
}),
ReviewDecision::ApprovedForAlways => {
additional_permissions.as_ref().map(|_| ApprovalOption {
label: "Yes, and always allow these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForAlways),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
})
}
ReviewDecision::ApprovedForAlways => None,
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => {
@@ -1289,6 +1282,37 @@ mod tests {
);
}
#[test]
fn additional_permissions_exec_options_hide_persistent_approval() {
let additional_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![absolute_path("/tmp/readme.txt")]),
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
..Default::default()
};
let options = exec_options(
&[
ReviewDecision::Approved,
ReviewDecision::ApprovedForSession,
ReviewDecision::ApprovedForAlways,
ReviewDecision::Abort,
],
None,
Some(&additional_permissions),
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
assert_eq!(
labels,
vec![
"Yes, proceed".to_string(),
"Yes, and allow these permissions for this session".to_string(),
"No, and tell Codex what to do differently".to_string(),
]
);
}
#[test]
fn permissions_options_use_expected_labels() {
let labels: Vec<String> = permissions_options()
codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs
+32 -8
View File
@@ -699,14 +699,7 @@ fn exec_options(
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
}),
ReviewDecision::ApprovedForAlways => {
additional_permissions.as_ref().map(|_| ApprovalOption {
label: "Yes, and always allow these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForAlways),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
})
}
ReviewDecision::ApprovedForAlways => None,
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => {
@@ -1275,6 +1268,37 @@ mod tests {
);
}
#[test]
fn additional_permissions_exec_options_hide_persistent_approval() {
let additional_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![absolute_path("/tmp/readme.txt")]),
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
..Default::default()
};
let options = exec_options(
&[
ReviewDecision::Approved,
ReviewDecision::ApprovedForSession,
ReviewDecision::ApprovedForAlways,
ReviewDecision::Abort,
],
None,
Some(&additional_permissions),
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
assert_eq!(
labels,
vec![
"Yes, proceed".to_string(),
"Yes, and allow these permissions for this session".to_string(),
"No, and tell Codex what to do differently".to_string(),
]
);
}
#[test]
fn permissions_options_use_expected_labels() {
let labels: Vec<String> = permissions_options()